Don't log API key #299
Don't log API key #299
Conversation
|
Thank you for the contribution, @jborrey. Taking a look. |
| @@ -181,7 +181,7 @@ def deliver | |||
| Bugsnag.warn "No API key configured, couldn't notify" | |||
| return | |||
| elsif api_key !~ API_KEY_REGEX | |||
| Bugsnag.warn "Your API key (#{api_key}) is not valid, couldn't notify" | |||
There was a problem hiding this comment.
This instance should probably stay; since the API key does not match the required format, its useful to compare the logged output to your actual key to correct typos. There is no data exposure for logging an incorrect key.
There was a problem hiding this comment.
Do you think that it is still a dangerous log line, say if someone accidentally set the Bugsnag API key to another API key or malformed API key?
Would the message not give enough information without logging the API key?
If not, I'll not remove this log line but would prefer to.
There was a problem hiding this comment.
It's not dangerous at all to keep it logged, really. The API key cannot be used to access your data in any way. It does seem excessive however that it is logged everywhere always.
[This PR](bugsnag@5eaddd8) was a good start but there was more. Used to find where more changes were needed ```bash ~/src/bugsnag-ruby (master)$ grep -r "#{api_key}" . ./lib/bugsnag/notification.rb: Bugsnag.warn "Your API key (#{api_key}) is not valid, couldn't notify" ./lib/bugsnag/notification.rb: Bugsnag.log("Notifying #{endpoint} of #{@exceptions.last.class} from api_key #{api_key}") ``` Should not log API key since logs are not necessarily kept as secret a environment variables. The log steam should be keeping track of which service/box is doing the logging in order to identify which client of the API is having issues. We have kept the logging of the variable for an invalid API key.
jborrey
force-pushed
the
jborrey/remove-logging-of-api-key
branch
from
e006a0f
to
8c96928
Compare
|
@kattrali Updated so it only removes on instance of the API key logging. |
|
Thanks! Releasing this in a patch shortly. |
This PR was a good start but there was more.
Used to find where more changes were needed
Should not log API key since logs are not necessarily kept as secret a
environment variables. The log steam should be keeping track of which
service/box is doing the logging in order to identify which client of
the API is having issues.
Result of running spec: