Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unmarshalling error logs contain Slack user data #579

Closed
richgerrard opened this issue May 16, 2022 · 2 comments · Fixed by #583
Closed

Unmarshalling error logs contain Slack user data #579

richgerrard opened this issue May 16, 2022 · 2 comments · Fixed by #583
Assignees
Labels
bug Something isn't working
Projects
Milestone

Comments

@richgerrard
Copy link
Contributor

Describe the bug
When the dependency (nlopes/slack) can't marshall a message event, an error is logged.
The log contains information about the event, which contains much too much information on the Slack user.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Slack
  2. Click on the huddle icon; do it in a really public channel just to annoy lots of other people with your test. Some of them might join to say hi! Please tell them I said: "hello".
  3. Dump the error logs of your botKube pod (k logs botkube-blah)
  4. See error
Slack unmarshalling error: RTM Error: Received unmapped event "user_huddle_changed": {all my info about me}

Expected behavior
Don't log things that don't matter.

Screenshots
n/a

Additional context
nlopes/slack has moved to slack-go/slack, and you should probably also update your dependency, but the problem with using this tool is that it listens to every possible slack event.
(ref: https://github.com/slack-go/slack/blob/0e14c9d4a15c91e845bc371fe91ee58763a3813a/websocket_managed_conn.go#L494)
Which means BotKube listens to every event
Which makes this tool a really frightening kind of slack spy program that's running in my kubernetes cluster and outputting employee information in error logs.

@richgerrard richgerrard added the bug Something isn't working label May 16, 2022
@PrasadG193
Copy link
Collaborator

Thanks for reporting this. We will prioritize this for the upcoming release

@pkosiec pkosiec self-assigned this May 19, 2022
@pkosiec pkosiec changed the title [BUG] Unmarshalling error logs contain slack user data Unmarshalling error logs contain Slack user data May 19, 2022
@pkosiec
Copy link
Member

pkosiec commented May 19, 2022

Hi @richgerrard,

Thanks for describing the issue. I've checked that, and you're absolutely right—it is caused by the slack-go/slack library. I've prepared a PR with fix directly to the repo: slack-go/slack#1067

Also, to avoid being blocked, I've created a PR with a workaround on BotKube side: #583. I hope it will be merged soon 🤞

Cheers!

@pkosiec pkosiec added this to the v0.13.0 milestone May 20, 2022
@pkosiec pkosiec added this to To do in v0.13.0 via automation May 20, 2022
@pkosiec pkosiec moved this from To do to To review in v0.13.0 May 20, 2022
@mergify mergify bot closed this as completed in #583 May 26, 2022
v0.13.0 automation moved this from To review to Done May 26, 2022
mergify bot pushed a commit that referenced this issue May 26, 2022
##### ISSUE TYPE
 - Bug fix Pull Request

##### SUMMARY

- Do not log full Slack event details in case of unmarshalling error

This is an issue of the upstream library. While I prepared a workaround, I also prepared a simple change in `slack-go/slack` repo: slack-go/slack#1067

**NOTE:** This pull request depends on #582. Please merge #582 first, and I will rebase this one.

Actual changes on this PR: 7813821

Fixes #579

##### TESTING

See the original issue for testing instruction: #579
You can use `pkosiec/botkube:slack-unmarshall-err` Docker image for testing the workaround.
Once you run my image and try to reproduce the issue, you'll see in the logs:

```
ERRO[2022-05-19T16:00:57Z] Slack unmarshalling error: RTM Error: Received unmapped event "user_huddle_changed"
ERRO[2022-05-19T16:00:57Z] Slack unmarshalling error: RTM Error: Received unmapped event "sh_room_join"
ERRO[2022-05-19T16:01:00Z] Slack unmarshalling error: RTM Error: Received unmapped event "sh_room_leave"
```
mergify bot pushed a commit that referenced this issue Jun 6, 2022
… details from logs #610

##### ISSUE TYPE
 - Feature Pull Request
 - Bug fix Pull Request
 - Docs Pull Request

##### SUMMARY

~⚠️ **NOTE:** For convienence, this PR depends on #603 and needs to be rebased when #603 is merged (it should happen very soon). Actual changes: a6d91f21f25f00d679572e62b45cd2396d571075~

As the upstream fix has been merged (slack-go/slack#1067), this PR updates `slack-go/slack` dependency and removes no longer needed workaround.

See the original issue: #579
See the PR that introduced the workaround: #583

##### TESTING

> **NOTE:** You can also use `pkosiec/botkube:rm-slack-workaround` Docker image for testing. The image was repushed **after** rebase.

1. Checkout this PR
2. Edit `comm_config.yaml` and enable Slack
3. Export needed envs:

```bash
export KUBECONFIG=/Users/$USER/.kube/config
export LOG_LEVEL=info
```

4. Run BotKube locally:

```bash
go run ./cmd/botkube/main.go
```

5. Start Slack huddle and say something, then leave
6. Observe logs still without event details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants