Remove dangerous format specifiers #1566
Conversation
theunrepentantgeek
requested review from
babbageclunk,
davefellows,
matthchr and
Porges
as code owners
| @@ -112,7 +112,13 @@ func TestOwnerNotFound_RemembersCause(t *testing.T) { | |||
|
|
|||
| g.Expect(errors.Cause(err)).To(Equal(cause)) | |||
|
|
|||
| fmtedErr := fmt.Sprintf("%+v", err) | |||
| var builder strings.Builder | |||
| for err != nil { | |||
There was a problem hiding this comment.
I don't think this loop is necessary - the wrapper error should include the context of the errors it wraps. The stacktrace that's needed for the error line assertions can be retrieved using this technique.
There was a problem hiding this comment.
The ReferenceNotFound error wasn't including the nested context, but now it does. I'll query the stack trace separately.
Codecov Report
@@ Coverage Diff @@
## master #1566 +/- ##
==========================================
- Coverage 63.48% 63.41% -0.07%
==========================================
Files 178 178
Lines 11739 11739
==========================================
- Hits 7452 7444 -8
- Misses 3620 3628 +8
Partials 667 667
Continue to review full report at Codecov.
|
|
|
||
| // stackTracer allows access to the stack trace of an error | ||
| // This should be exposed by the errors package, but it is not | ||
| type stackTracer interface { |
There was a problem hiding this comment.
It would make more sense to define this in the test since it's the only place that's using it.
What this PR does / why we need it:
Removes all uses of the format specifiers
%vand%+vas those dump entire objects into the log, which very likely will include secrets (e.g. credentials etc); we don't want those exposed in the logs.Closes #1470
Special notes for your reviewer:
How does this PR make you feel:
