-
Notifications
You must be signed in to change notification settings - Fork 839
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
fix: redact bot_access_tokens
from the debug logs of socket mode
#1519
fix: redact bot_access_tokens
from the debug logs of socket mode
#1519
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1519 +/- ##
=======================================
Coverage 84.89% 84.89%
=======================================
Files 112 113 +1
Lines 12473 12480 +7
=======================================
+ Hits 10589 10595 +6
- Misses 1884 1885 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! The change looks good to me but here is a minor suggestion on naming
import re | ||
|
||
|
||
def debug_message_redact(message: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that you followed this file and method naming: https://github.com/slackapi/bolt-python/blob/main/slack_bolt/logger/messages.py The format is more of "{log_level}_{log meaning}", thus "debug_redacted_message_string" or something like that would be even better. What do you think?
def debug_message_redact(message: str) -> str: | |
def debug_redacted_message_string(message: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed 👍 I like debug_redacted_message_string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
This PR modified the behavior of
socket mode
in order to redactbot_access_tokens
out of thedebug
logsSince the raw payload message is printed, I implemented a
regex
based solution, this feels less then idea for maintainability purposes, let me know if there are other better alternatives to thisCategory (place an
x
in each of the[ ]
)/docs-src
(Documents, have you run./scripts/docs.sh
?)/docs-src-v2
(Documents, have you run./scripts/docs-v2.sh
?)/tutorial
(PythOnBoardingBot tutorial)tests
/integration_tests
(Automated tests for this library)Requirements (place an
x
in each[ ]
)python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh
after making the changes.