Issue 844: Redact api token in chat message

briemarie commented 2 years ago

Debug logs are printing api tokens as part of the request message. Instead of printing the token, the token is transformed into a redacted string if it is not empty. If the token is empty, which is a useful piece of information that should be surfaced in the request, the empty string is transformed into a nil string to make it more obvious.

Pull Request Guidelines

These are recommendations for pull requests. They are strictly guidelines to help manage expectations.

PR preparation

[PASSED] Run make pr-prep from the root of the repository to run formatting, linting and tests.

Should this be an issue instead - YES IT IS #844

[ ] is it a convenience method? (no new functionality, streamlines some use case)
[X] exposes a previously private type, const, method, etc.
[ ] is it application specific (caching, retry logic, rate limiting, etc)
[ ] is it performance related.

API changes

Since API changes have to be maintained they undergo a more detailed review and are more likely to require changes.

no tests, if you're adding to the API include at least a single test of the happy case.
If you can accomplish your goal without changing the API, then do so.
dependency changes. updates are okay. adding/removing need justification.

Examples of API changes that do not meet guidelines:

in library cache for users. caches are use case specific.
Convenience methods for Sending Messages, update, post, ephemeral, etc. consider opening an issue instead.

briemarie commented 1 year ago

@christophercutajar I see you approved this. Will it be getting merged or was there another release that fixed the issue of the token being exposed in logs?

christophercutajar commented 1 year ago

@kanata2 when you have time can you please review it yourself and merge it!

github-actions[bot] commented 1 year ago

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

briemarie commented 1 year ago

kanata2 I realize that my proposed solution wasn't the proper one, but has this been solved internally by your team in another PR?

kanata2 commented 1 year ago

No, we have not worked on it yet. If you think the labeling by GitHub Actions is inappropriate, please re-open it and reply to my comment.

briemarie commented 1 year ago

@kanata2 Its ok that this PR is closed since it wasn't the right solution. Looks like the original issue https://github.com/slack-go/slack/issues/844 is still open so hopefully that gets addressed.

slack-go / slack