Repository: infracloudio/botkube (go)

Issue:
Unmarshalling error logs contain Slack user data
**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.


Pull request description:
Do not log full Slack event details in case of unmarshalling error
##### 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: https://github.com/slack-go/slack/pull/1067

**NOTE:** This pull request depends on #582. Please merge #582 first, and I will rebase this one.

Actual changes on this PR: https://github.com/infracloudio/botkube/commit/7813821947c6cbcc7d7040f3fb1a70ed57e9a4f6

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"
``` 


Patch:
diff --git a/pkg/bot/slack.go b/pkg/bot/slack.go
index 135c337f..a02890f0 100644
--- a/pkg/bot/slack.go
+++ b/pkg/bot/slack.go
@@ -112,7 +112,7 @@ func (b *SlackBot) Start() {
 			log.Errorf("Slack outgoing event error: %+v", ev.Error())
 
 		case *slack.UnmarshallingErrorEvent:
-			log.Errorf("Slack unmarshalling error: %+v", ev.Error())
+			log.Errorf("Slack unmarshalling error: %+v", b.stripUnmarshallingErrEventDetails(ev.Error()))
 
 		case *slack.RateLimitedError:
 			log.Errorf("Slack rate limiting error: %+v", ev.Error())
@@ -126,6 +126,27 @@ func (b *SlackBot) Start() {
 	}
 }
 
+// Temporary workaround until the PR is merged: https://github.com/slack-go/slack/pull/1067
+// There are just two cases when the full event details are concatenated with the error message: https://github.com/slack-go/slack/blob/v0.10.3/websocket_managed_conn.go#L474-L492
+// Also, we need keep the JSON unmarshal error intact: https://github.com/slack-go/slack/blob/v0.10.3/websocket_managed_conn.go#L405
+func (b *SlackBot) stripUnmarshallingErrEventDetails(errMessage string) string {
+	const errMsgSeparator = ": "
+	const prefix = "RTM Error"
+
+	if !strings.HasPrefix(errMessage, prefix) {
+		return errMessage
+	}
+
+	// has prefix, so let's split the message into parts
+	msgParts := strings.Split(errMessage, errMsgSeparator)
+	if len(msgParts) < 3 {
+		// impossible with current version of the dependency
+		return errMessage
+	}
+
+	return strings.Join(msgParts[:2], errMsgSeparator)
+}
+
 func (sm *slackMessage) HandleMessage(b *SlackBot) {
 	// Check if message posted in authenticated channel
 	info, err := sm.SlackClient.GetConversationInfo(sm.Event.Channel, true)


Test patch:
diff --git a/pkg/bot/export_test.go b/pkg/bot/export_test.go
new file mode 100644
index 00000000..9b84f884
--- /dev/null
+++ b/pkg/bot/export_test.go
@@ -0,0 +1,5 @@
+package bot
+
+func (b *SlackBot) StripUnmarshallingErrEventDetails(errMessage string) string {
+	return b.stripUnmarshallingErrEventDetails(errMessage)
+}
diff --git a/pkg/bot/slack_test.go b/pkg/bot/slack_test.go
new file mode 100644
index 00000000..8349c27e
--- /dev/null
+++ b/pkg/bot/slack_test.go
@@ -0,0 +1,53 @@
+package bot_test
+
+import (
+	"fmt"
+	"github.com/infracloudio/botkube/pkg/bot"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestSlackBot_StripUnmarshallingErrEventDetails(t *testing.T) {
+	//given
+	sampleEvent := `{"type":"user_huddle_changed","user":{"id":"id","team_id":"team_id"}, "event_ts":"1652949120.004700"}`
+
+	testCases := []struct {
+		Name     string
+		Input    string
+		Expected string
+	}{
+		{
+			Name:     "Unmapped event",
+			Input:    fmt.Sprintf(`RTM Error: Received unmapped event "user_huddle_changed": %s`, sampleEvent),
+			Expected: `RTM Error: Received unmapped event "user_huddle_changed"`,
+		},
+		{
+			Name:     "Unmarshalling error message",
+			Input:    fmt.Sprintf(`RTM Error: Could not unmarshall event "user_huddle_changed": %s`, sampleEvent),
+			Expected: `RTM Error: Could not unmarshall event "user_huddle_changed"`,
+		},
+		{
+			Name:     "JSON unmarshal error",
+			Input:    "cannot unmarshal bool into Go value of type string",
+			Expected: "cannot unmarshal bool into Go value of type string",
+		},
+		{
+			Name: "JSON unmarshal error with colons",
+			// this is a real error when doing json.Unmarshal([]byte(`":::"`), &time)
+			Input:    `parsing time "":::"" as ""2006-01-02T15:04:05Z07:00"": cannot parse ":::"" as "2006"`,
+			Expected: `parsing time "":::"" as ""2006-01-02T15:04:05Z07:00"": cannot parse ":::"" as "2006"`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			slackBot := &bot.SlackBot{}
+
+			// when
+			actual := slackBot.StripUnmarshallingErrEventDetails(testCase.Input)
+
+			// then
+			assert.Equal(t, testCase.Expected, actual)
+		})
+	}
+}