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

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

Merged

Conversation

pkosiec
Copy link
Member

@pkosiec pkosiec commented May 19, 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"

@pkosiec pkosiec added bug Something isn't working hold-off-merging labels May 19, 2022
@pkosiec pkosiec force-pushed the slack-unmarshall-err-remove-event-details branch from 9256d5b to 7813821 Compare May 20, 2022 10:12
@pkosiec pkosiec force-pushed the slack-unmarshall-err-remove-event-details branch from 7813821 to d2ce098 Compare May 25, 2022 08:25
Copy link
Collaborator

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mergify mergify bot merged commit 0a2bf80 into kubeshop:develop May 26, 2022
mergify bot pushed a commit that referenced this pull request 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
mergify bot pushed a commit that referenced this pull request Jun 21, 2022
##### ISSUE TYPE

 - Feature Pull Request

##### SUMMARY

Add an option to push the BotKube image automatically on PR. It's alternative approach for #604.

This PR will solve the problem with manual PR builds, e.g. we had that issue here:
- #601
- #593
- #582
- #583

Example run: https://github.com/mszostok/botkube/runs/6714112689?check_suite_focus=true

Fixes #590 

To ensure that secrets won't be available for untrusted code, first we need to build the image and share it with the second job, which doesn't check out the untrusted code and can safely push an artifact to ghcr.io.

The flow is as follows:
```
Job1: image build -> image save -> artifact upload 
Job2: artifact download -> image load -> image push
```
Job1—runs untrusted code but without write repo perms
Job2—push built image with package write perms

#### Security

This article describes it well: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
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
None yet
Development

Successfully merging this pull request may close these issues.

Unmarshalling error logs contain Slack user data
2 participants