From 370c56ec9e7e9e9a097353187641d013cd0de962 Mon Sep 17 00:00:00 2001
From: Tobias Gesellchen <tobias@gesellix.de>
Date: Mon, 25 May 2026 10:34:13 +0200
Subject: [PATCH] fix(security): remove credential-log bypass and sanitise
 header values in proxy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two alerts at proxy.go:87:

- go/clear-text-logging (alert 294): the UnsafeLogCredentialHeaders escape
  hatch allowed credential-bearing headers (Authorization, Cookie, …) to
  reach log.Printf in plaintext when LOG_PROXY_CREDENTIALS=true. CodeQL
  traces the taint regardless of the conditional.

  Remove UnsafeLogCredentialHeaders entirely. The field, env-var init, and
  the 'No redaction' branch in formatHeaders are all deleted. Credentials
  are now always redacted unconditionally. Developers who need to inspect
  live credentials can use a tool like mitmproxy or Wireshark instead.

- go/log-injection (alert 295): header values assembled by formatHeaders
  were passed to log.Printf without newline stripping, allowing a
  malicious response to inject fake log lines.

  Apply sanitizeLog(val) to every non-redacted header value before it is
  added to the string builder. Redacted values stay as the literal string
  "[REDACTED]" which needs no further sanitisation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 pkg/service/proxy/proxy.go | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/pkg/service/proxy/proxy.go b/pkg/service/proxy/proxy.go
index ee31b0d..4f0da4b 100644
--- a/pkg/service/proxy/proxy.go
+++ b/pkg/service/proxy/proxy.go
@@ -36,25 +36,15 @@ type LoggingProxy struct {
 	RecordEnabled bool
 	MaxBodySize   int64
 	Recorder      *Recorder
-
-	// UnsafeLogCredentialHeaders disables the otherwise-unconditional
-	// redaction of credential-bearing headers (Authorization, Cookie, …) in
-	// LogRequest / LogResponse output. This is an explicit
-	// "I-know-what-I'm-doing" escape hatch for local debugging only — never
-	// enable it in production. Defaults to false; the env-var
-	// LOG_PROXY_CREDENTIALS=true flips it on so a developer can opt in
-	// without recompiling.
-	UnsafeLogCredentialHeaders bool
 }
 
 // NewLoggingProxy creates a lightweight logger for HTTP requests/responses.
 func NewLoggingProxy(_ string, redact bool) *LoggingProxy {
 	// targetURL logic should be handled by the caller or we can parse it here
 	return &LoggingProxy{
-		Redact:                     redact,
-		LogBody:                    os.Getenv("LOG_PROXY_BODY") == "true",
-		UnsafeLogCredentialHeaders: os.Getenv("LOG_PROXY_CREDENTIALS") == "true",
-		MaxBodySize:                1024 * 10, // 10KB default limit for logging
+		Redact:      redact,
+		LogBody:     os.Getenv("LOG_PROXY_BODY") == "true",
+		MaxBodySize: 1024 * 10, // 10KB default limit for logging
 	}
 }
 
@@ -65,7 +55,7 @@ func (lp *LoggingProxy) SetRecorder(r *Recorder) {
 
 // LogRequest prints an abbreviated request with optional header/body redaction.
 func (lp *LoggingProxy) LogRequest(r *http.Request) {
-	headers := formatHeaders(r.Header, lp.Redact, lp.UnsafeLogCredentialHeaders)
+	headers := formatHeaders(r.Header, lp.Redact)
 
 	bodyStr := "[HIDDEN]"
 
@@ -89,7 +79,7 @@ func (lp *LoggingProxy) LogRequest(r *http.Request) {
 
 // LogResponse prints an abbreviated response with optional header/body redaction.
 func (lp *LoggingProxy) LogResponse(r *http.Response) {
-	headers := formatHeaders(r.Header, lp.Redact, lp.UnsafeLogCredentialHeaders)
+	headers := formatHeaders(r.Header, lp.Redact)
 
 	bodyStr := "[HIDDEN]"
 
@@ -115,24 +105,24 @@ func (lp *LoggingProxy) LogResponse(r *http.Response) {
 	}
 }
 
-func formatHeaders(h http.Header, redact, unsafeLogCredentials bool) string {
+func formatHeaders(h http.Header, redact bool) string {
 	var sb strings.Builder
 	// In Go, http.Header is a map[string][]string.
 	// Iterating over the map directly allows us to see the actual keys
 	// stored in the map, which might not be canonical if set directly.
 	for k, vv := range h {
 		val := strings.Join(vv, ", ")
-		// Credentials (Authorization, Cookie, …) are redacted by default.
-		// unsafeLogCredentials lifts that floor entirely — explicit opt-in
-		// for local debugging only. When the floor is in place, the
-		// caller's broader Redact toggle adds further coverage.
+		// Credential-bearing headers are always redacted; the broader Redact
+		// toggle covers additional sensitive fields. Header values are passed
+		// through sanitizeLog to strip any embedded newlines before they reach
+		// the log sink (go/log-injection, alert 295).
 		switch {
-		case unsafeLogCredentials:
-			// No redaction.
 		case isAlwaysSensitive(k):
 			val = "[REDACTED]"
 		case redact && isSensitive(k):
 			val = "[REDACTED]"
+		default:
+			val = sanitizeLog(val)
 		}
 
 		fmt.Fprintf(&sb, "    %s: %s\n", k, val)