Obfuscates ACL tokens appearing in /v1/acl/<verb>/<token> APIs. #3276
Conversation
slackpad
changed the title
| @@ -166,6 +167,12 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler { | |||
| return mux | |||
| } | |||
|
|
|||
| // aclEndpointRE is used to find old ACL endpoints that take tokens in the URL | |||
There was a problem hiding this comment.
I think the RE is too clever. I've read this a couple of times now and it isn't clear to me what it is trying to achieve. You could try to use multiple REs or at least provide some examples of what this is supposed to match. Since you're trying to obfuscate sensitive data I'd say clarity goes before efficiency. Maybe don't use an RE at all and work with url.Parse or strings.Split.
agent/http_test.go
Outdated
| a.srv.wrap(handler)(resp, req) | ||
|
|
||
| // Make sure no tokens from the URL show up in the log | ||
| if strings.Contains(buf.String(), "secret") { |
There was a problem hiding this comment.
buf.String() allocs twice here. Not relevant for the test but don't leave a bad example. Also, don't you need to reset the buffer between test runs? Probably not that important here. secret should never show up.
There was a problem hiding this comment.
The buffer will accumulate. I changed this to look for a specific string in the buffer, which is a lot safer. Also fixed the double string call.
Here's how this looks after the change, note that the URL-based tokens are now redacted, too: