-
Notifications
You must be signed in to change notification settings - Fork 3
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
test: added more code coverage for usecase package #342
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 68.61% 72.41% +3.80%
==========================================
Files 64 64
Lines 4320 4376 +56
==========================================
+ Hits 2964 3169 +205
+ Misses 1111 949 -162
- Partials 245 258 +13 ☔ View full report in Codecov by Sentry. |
50211e3
to
e66e69a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good Ethan, my only ask is if we can add bounds checking to the overflow errors. Our security folks won't like us ignoring possible overflow integer errors.
I'm thinking we can do something like:
lenChallengeUsername := uint8(0)
lenAuthURL := uint8(0)
// Check for potential overflow
if len(challenge.Username) <= math.MaxUint8 {
lenChallengeUsername = uint8(len(challenge.Username))
}
if len(authURL) <= math.MaxUint8 {
lenChallengeUsername = uint8(len(challenge.Username))
}
That seems to be a way to address the findings: securego/gosec#1187 . It may not get rid of the errors since that was merged so recently, but at least can get the code in place.
8dc0507
to
09b9d62
Compare
014b171
to
db81d73
Compare
762fbef
to
5b9030a
Compare
added more code coverage for usecase package