Redact sensitive info before logging build messages #2003
Conversation
|
|
||
| func (m *WebsocketMessages) Send(msg *Message) error { | ||
| logrus.Tracef("Send build message %s", msg) | ||
| logrus.Tracef("Send build message %s", redact(msg)) |
There was a problem hiding this comment.
This could probably be achieved through a custom logrus formatter as well, but I think this is concise and works well enough.
There was a problem hiding this comment.
Yeah, no need to right now unless you can think of other places that need redactions.
pkg/buildclient/messages.go
Outdated
| // redact removes sensitive information from a Message. | ||
| // Use this before logging a message. | ||
| func redact(msg *Message) *Message { | ||
| redacted := z.Dereference(msg) |
There was a problem hiding this comment.
This is really a gratuitous use of z.Deference, don't use this in place of a if msg != nil. Code tricks obscure the intention and decrease readibility. z.Dereference is mostly acceptable when dealing it native types (*int, *string, *bool) because they are very unnatural to interact with, but a struct pointer shouldn't fall into that bucket.
There was a problem hiding this comment.
That's reasonable, I'll amend this to something like
var redacted Message
if msg != nil {
redacted = *msg
}and update the godocs for z.Dereference to prevent future misuse
This prevents credential leakage when using higher log levels during builds. Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
4 participants
This prevents credential leakage when using higher log levels during builds.
Addresses https://github.com/acorn-io/manager/issues/828