slack icon indicating copy to clipboard operation
slack copied to clipboard

OAuth Slack Token Logged In Clear

Open christophercutajar opened this issue 4 years ago • 10 comments

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, https://github.com/slack-go/slack/blob/686c209f9525a78313cfe54cbc07cd86bd677384/chat.go#L217 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: https://github.com/argoproj/argo-events/issues/944

christophercutajar avatar Nov 09 '20 21:11 christophercutajar

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?

kanata2 avatar Nov 10 '20 00:11 kanata2

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.

christophercutajar avatar Nov 10 '20 13:11 christophercutajar

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

kanata2 avatar Nov 10 '20 17:11 kanata2

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

christophercutajar avatar Nov 10 '20 18:11 christophercutajar

Thanks!

kanata2 avatar Nov 11 '20 05:11 kanata2

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

kanata2 avatar Apr 18 '21 16:04 kanata2

@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.

briemarie avatar Aug 23 '22 20:08 briemarie

@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 :(

christophercutajar avatar Aug 24 '22 09:08 christophercutajar

@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. https://github.com/slack-go/slack/pull/1102

briemarie avatar Aug 24 '22 16:08 briemarie

Thanks @briemarie! I'll confirm later.

kanata2 avatar Aug 25 '22 02:08 kanata2

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

qwerty1q2w avatar Apr 21 '23 08:04 qwerty1q2w

I sent now a proposed simpler fix: #1215

daabr avatar Jul 14 '23 23:07 daabr