Open nedal87 opened 6 months ago
This is in the upstream lib here: https://github.com/argoproj/notifications-engine/blob/master/pkg/util/http/logroundtripper.go#L21
Hi @zachaller, Thanks for sharing the URL for the upstream lib. While I understand that the current behavior is by design, I wanted to emphasize the importance of mitigating potential security risks associated with logging sensitive data. Specifically, Slack tokens and webhook URLs can pose a significant security threat if leaked. This was the primary motivation behind creating the bug report.
What about masking such sensitive data in the logs?
I faced a similar issue with our notifications proxy application and added some code to our logger which would redact this kind of information. See example snippet below:
func convertRequestHeadersToJSON(r *http.Request) string {
if r == nil {
return ""
}
// Redact certain headers
redactHeaders := r.Header.Clone()
if redactHeaders.Get("Authorization") != "" {
redactHeaders.Set("Authorization", "Bearer xxxxx")
}
if redactHeaders.Get("Cookie") != "" {
redactHeaders.Set("Cookie", "Redacted away")
}
headers, err := json.MarshalIndent(redactHeaders, "", "")
if err != nil || string(headers) == "null" {
return ""
}
return strings.ReplaceAll(string(headers), "\n", "")
}
Any news here ?
Checklist:
Describe the bug
There is a potential leakage of sensitive data in the application logs of Argo Rollouts. When the controller logs is set to debug mode or encounters exceptions, it may inadvertently log HTTP requests containing sensitive data, including authentication tokens and webhook URLs, as seen in the following log message, the slack token is logged as is.
time="2024-02-27T11:09:38Z" level=debug msg="Sending request: POST /api/chat.postMessage HTTP/1.1\r\nHost: slack.com\r\nContent-Type: application/x-www-form-urlencoded\r\n\r\nattachments=%5B%7B%22color%22%3A%22%2318be52%22%2C%22title%22%3A%22rollouts-demo%22%2C%22fields%22%3A%5B%7B%22title%22%3A%22Strategy%22%2C%22value%22%3A%22Canary%22%2C%22short%22%3Atrue%7D%2C%7B%22title%22%3A%22rollouts-demo%22%2C%22value%22%3A%22argoproj%2Frollouts-demo%3Ablue%22%2C%22short%22%3Atrue%7D%5D%2C%22blocks%22%3Anull%7D%5D&channel=%23testing-argo&text=Rollout+rollouts-demo+has+been+completed.&token=xoxb-xxxxxxxxxxxxxxxxxxx" service=slack
To Reproduce
Expected behavior
Sensitive data should not be exposed or it should be Redacted in the controller logs.
Screenshots
Version v1.66
Logs
Message from the maintainers:
Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.