OAuth Slack Token Logged In Clear

[christophercutajar]

Describe the bug

While using Argo Event which leverage slack-go/slack for Slack triggers, it was noticed that the OAuth Slack token is logged in the clear. Specifically,

slack/chat.go

Line 217 in 686c209

api.Debugf("Sending request: %s", string(reqBody))

is logging the whole API request resulting in also logging a clear version of the OAuth slack token in the log files. The following is an example:

sensor-qrlrb-58d497d595-q9dmh main 2020-11-09T10:11:50.147597554Z slack-go/slack2020/11/09 10:11:50 chat.go:217: Sending request: channel=random&text=webhook+triggered%21&token=xoxb-xxxx-xxx-xxx

Expected behavior

OAuth Slack token not never be logged in the clear in the log files.

Argo Events related issue: argoproj/argo-events#944

[kanata2]

Thanks for reporting.
As you said in argoproj/argo-events#944, api.Debugf writes only if debug mode is enabled.
So I think it's not a bug 🤔
What do you think?

[christophercutajar]

For the Argo part, I agree with you, the api.Debugf should be false.

In general, my personal opinion is that I don't think such token should be logged in the log files even in debug mode.

[kanata2]

Okay, it seems good to me. Please send a PR?

[christophercutajar]

@kanata2 will be able to submit a PR in the coming 2 days.

[kanata2]

Thanks!

[kanata2]

@christophercutajar I'll close it once, but feel free to re-open it or send us a pull request if you want!

[briemarie]

@kanata2 We're seeing the same behavior of the token being printed out into our logs. We've updated to the latest version as of today v0.11.2 but still getting the printout. We have these debug logs being stored in dashboards so this is a major problem. Has it been fixed? It is never ideal to have tokens printed in any logs, debug or dev or whatever.

[christophercutajar]

@kanata2 my apologies I wasn't able to create a PR for this. Can you re-open the ticket please as not I'm able to work on this.

@briemarie this wasn't solved from my end :(

[briemarie]

@christophercutajar and @kanata2 I opened a PR on a fork for you to review. Not sure if this is the kind of approach that is best or if the token should instead not be passed into the method calls, but I think this is a good approach since it is useful to know if a token was supplied or if it was empty.
#1102

[kanata2]

Thanks @briemarie! I'll confirm later.

[qwerty1q2w]

Hi! +1 The same problems. Token in logs.

[daabr]

I sent now a proposed simpler fix: #1215