Sensitive Data Leakage in Argo Rollouts Logs

[nedal87]

Checklist:

• [x] I've included steps to reproduce the bug.
• [x] I've included the version of argo rollouts.

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

1. Enable debug logging in Argo Rollouts.
2. Trigger actions that involve sending requests containing sensitive data, such as Slack webhook URLs or tokens.
3. Check the debug logs for the sent requests.

Expected behavior

Sensitive data should not be exposed or it should be Redacted in the controller logs.

Screenshots

Version v1.66

Logs

# Paste the logs from the rollout controller

# Logs for the entire controller:

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

---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

[zachaller]

This is in the upstream lib here: https://github.com/argoproj/notifications-engine/blob/master/pkg/util/http/logroundtripper.go#L21

[nedal87]

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?

[newtondev]

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", "")
}

[robert-heinzmann-logmein]

Any news here ?