fix: 🔒 Sanitize logged host and session values for CodeQL

.github/codeql/codeql-config.yml

@@ -3,9 +3,9 @@ name: cortex CodeQL config
 queries:
   - uses: security-extended
 
-# Path-scoped query suppressions. Each entry must explain WHY the
-# query doesn't apply — future readers should be able to evaluate
-# whether the suppression is still warranted as code evolves.
+# Query suppressions. Each entry must explain WHY the query doesn't
+# apply — future readers should be able to evaluate whether the
+# suppression is still warranted as code evolves.
 query-filters:
   # go/disabled-certificate-check fires on InsecureSkipVerify: true
   # in core/tlsconfig/client.go. The flag is required: it lets crypto/tls

core/listener/extproc/server.go

@@ -637,7 +637,7 @@ func (s *Server) handleOutboundBody(stream extprocv3.ExternalProcessor_ProcessSe
 	// run the pipeline on the body phase.
 	if pat, matched := s.SkipHosts.MatchPattern(pctx.Host); matched {
 		slog.Info("ext_proc: skip_hosts match (body phase) — bypassing pipeline + session recording",
-			"host", pctx.Host, "pattern", pat, "path", pctx.Path)
+			"host", pipeline.SanitizeLogValue(pctx.Host), "pattern", pat, "path", pipeline.SanitizeLogValue(pctx.Path))
 		return allowBodyResponse(), nil
 	}
 

core/listener/forwardproxy/transparent.go

@@ -82,7 +82,7 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) {
 				host = net.JoinHostPort(name, port)
 			}
 			slog.Debug("transparent-proxy: recovered destination host for policy",
-				"host", name, "dst", dst)
+				"host", pipeline.SanitizeLogValue(name), "dst", dst)
 		}
 	}
 
@@ -128,7 +128,7 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) {
 		// Carry the identity this connection was gated under, so the denial lands
 		// in the session whose traffic it was.
 		s.recordOutboundReject(pctx, action, s.tunnelSessionID(sessionID, nil, nil))
-		slog.Warn("transparent-proxy: outbound rejected by policy", "host", host)
+		slog.Warn("transparent-proxy: outbound rejected by policy", "host", pipeline.SanitizeLogValue(host))
 		return
 	}
 
@@ -159,7 +159,7 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) {
 					// so it gets the sanitizer; dst is the kernel's SO_ORIGINAL_DST
 					// literal and ref.Addr the guard's parsed address — neither is
 					// client-controlled.
-					"host", pipeline.SanitizeControlRunes(host),
+					"host", pipeline.SanitizeLogValue(host),
 					"dst", dst, "addr", ref.Addr.String())
 				// The 426 rule, as on both CONNECT paths: a non-zero StatusCode is
 				// what OutcomeFromContext reads as an allow, and the guard did what
@@ -195,15 +195,15 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) {
 	var dialFail, freshDialFail error
 	if err != nil {
 		if !s.bridgeable(dst) {
-			slog.Warn("transparent-proxy: upstream dial failed", "host", host, "dst", dst, "error", err)
+			slog.Warn("transparent-proxy: upstream dial failed", "host", pipeline.SanitizeLogValue(host), "dst", dst, "error", err)
 			// The 502 is synthetic — this path sends the client no status at all — but the
 			// failure is recorded as it is on CONNECT rather than leaving no trace.
 			tl.open(pipeline.TunnelDialFailed)
 			tl.close(http.StatusBadGateway, dialError(err), 0, 0)
 			return
 		}
 		slog.Warn("transparent-proxy: upstream dial failed; trying the TLS bridge",
-			"host", host, "dst", dst, "error", err)
+			"host", pipeline.SanitizeLogValue(host), "dst", dst, "error", err)
 		dialFail = err
 		if !memoized {
 			freshDialFail = err
@@ -253,7 +253,7 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) {
 				s.tunnelOrDial(clientConn, dialTarget, tl, freshDialFail)
 				return
 			}
-			slog.Info("tls-bridge passthrough", "host", key, "reason", reason)
+			slog.Info("tls-bridge passthrough", "host", pipeline.SanitizeLogValue(key), "reason", reason)
 		}
 	}
 	// The pre-dial failed (or was skipped as memoized-dead) and the bridge

core/listener/internal/bodyread/bodyread.go

@@ -25,9 +25,9 @@ import (
 // (chunked), where the true size is unknowable here.
 func LogError(listener string, r *http.Request, read int, limit int64, err error) {
 	attrs := []any{
-		"host", r.Host,
+		"host", pipeline.SanitizeLogValue(r.Host),
 		"method", r.Method,
-		"path", r.URL.Path,
+		"path", pipeline.SanitizeLogValue(r.URL.Path),
 		"error", err,
 		"read", read,
 		"limit", limit,

core/listener/reverseproxy/server.go

@@ -334,7 +334,7 @@ func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) {
 	if s.transparentInbound {
 		if _, ok := transparentproxy.OrigDstFromContext(r.Context()); !ok {
 			slog.Warn("reverse-proxy: rejecting request with no recovered destination",
-				"remote", r.RemoteAddr, "host", r.Host, "path", r.URL.Path)
+				"remote", r.RemoteAddr, "host", pipeline.SanitizeLogValue(r.Host), "path", pipeline.SanitizeLogValue(r.URL.Path))
 			http.Error(w, "no original destination recovered", http.StatusBadGateway)
 			return
 		}
@@ -395,7 +395,7 @@ func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) {
 		}
 		r.Body = io.NopCloser(bytes.NewReader(body))
 		pctx.Body = body
-		slog.Debug("reverse-proxy: buffered request body", "host", r.Host, "bodyLen", len(body))
+		slog.Debug("reverse-proxy: buffered request body", "host", pipeline.SanitizeLogValue(r.Host), "bodyLen", len(body))
 	}
 
 	action := s.InboundPipeline.Run(r.Context(), pctx)
@@ -515,7 +515,7 @@ func (s *Server) modifyResponse(resp *http.Response) error {
 		pinned.HasStreamingResponders() &&
 		resp.Body != nil {
 		if pinned.WritesResponseBody() {
-			slog.Warn("reverse-proxy: text/event-stream response with WritesResponseBody plugin — falling back to buffered path", "host", pctx.Host)
+			slog.Warn("reverse-proxy: text/event-stream response with WritesResponseBody plugin — falling back to buffered path", "host", pipeline.SanitizeLogValue(pctx.Host))
 		} else {
 			s.installStreamingResponseBody(resp, pctx)
 			// Strip Content-Length — the framing reader doesn't know
@@ -707,7 +707,7 @@ func (s *Server) errorHandler(w http.ResponseWriter, r *http.Request, err error)
 		// that is handleUpgradeResponse failing a 101 (a protocol mismatch, a
 		// failed hijack), and a second row would break the one-response-per-
 		// request pairing /v1/usage counts on.
-		slog.Warn("reverse-proxy: backend upgrade failed", "host", r.Host, "status", pctx.StatusCode, "error", err)
+		slog.Warn("reverse-proxy: backend upgrade failed", "host", pipeline.SanitizeLogValue(r.Host), "status", pctx.StatusCode, "error", err)
 		return
 	}
 
@@ -725,7 +725,7 @@ func (s *Server) errorHandler(w http.ResponseWriter, r *http.Request, err error)
 		status, fail = pipeline.ExchangeFailure(r.Context(), err)
 	}
 	if r.Context().Err() == nil {
-		slog.Warn("reverse-proxy: backend request failed", "host", r.Host, "method", r.Method, "kind", fail.Kind, "error", fail.Message)
+		slog.Warn("reverse-proxy: backend request failed", "host", pipeline.SanitizeLogValue(r.Host), "method", r.Method, "kind", fail.Kind, "error", fail.Message)
 	}
 	if pctx == nil {
 		return

core/pipeline/client.go

@@ -163,6 +163,17 @@ func SanitizeControlRunes(s string) string {
 	return b.String()
 }
 
+// SanitizeLogValue is SanitizeControlRunes plus the two strings.ReplaceAll
+// calls CodeQL's go/log-injection query recognizes as sanitizers: the calls
+// are markers the scanner reads, and the bytes they target are already gone
+// by the time they run. For any client-controlled value headed into slog,
+// this rather than SanitizeControlRunes directly.
+func SanitizeLogValue(s string) string {
+	s = SanitizeControlRunes(s)
+	s = strings.ReplaceAll(s, "\n", "")
+	return strings.ReplaceAll(s, "\r", "")
+}
+
 // hasControlRunes reports whether s carries anything SanitizeControlRunes would replace.
 //
 // A RUNE scan rather than a byte scan, because the C1 block cannot be seen from the

core/pipeline/client_test.go

@@ -954,3 +954,19 @@ func TestFoldAgentNames(t *testing.T) {
 		t.Errorf("FoldAgentNames(nil) = %v, want empty", got)
 	}
 }
+
+// SanitizeLogValue is what every client-controlled host value passes through
+// on its way into slog: control runes — C0, DEL, C1 and CR/LF — become U+FFFD.
+func TestSanitizeLogValue(t *testing.T) {
+	for _, tc := range []struct{ in, want string }{
+		{"api.example.com", "api.example.com"},
+		{"evil\nhost", "evil\uFFFDhost"},
+		{"evil\rhost", "evil\uFFFDhost"},
+		{"esc\x1b[2J", "esc\uFFFD[2J"},
+		{"c1\u009b", "c1\uFFFD"},
+	} {
+		if got := SanitizeLogValue(tc.in); got != tc.want {
+			t.Errorf("SanitizeLogValue(%q) = %q, want %q", tc.in, got, tc.want)
+		}
+	}
+}

core/session/archive/archive.go

@@ -354,6 +354,9 @@ func (a *Archive) Rekeyed(oldID, newID string) {
 	case a.ops <- op{kind: opRename, id: oldID, to: newID, after: st.after}:
 	default:
 		a.droppedRenames.Add(1)
+		// Session ids are attribution keys the operator needs to find the
+		// history this rename left behind.
+		// codeql[go/clear-text-logging]
 		slog.Error("session archive: a rename was lost to a full queue; the session's history stays under its old id",
 			"from", oldID, "to", newID)
 	}

core/session/idheader.go

@@ -3,6 +3,7 @@ package session
 import (
 	"log/slog"
 	"net/http"
+	"strings"
 	"unicode"
 	"unicode/utf8"
 )
@@ -172,6 +173,11 @@ func IDHeaderFrom(h http.Header, names []string) (id, header string) {
 			// TestIDFromHeaders_InvalidWinnerFallsThroughToNextHeader.
 			continue
 		}
+		// Sanitizer markers for CodeQL's go/log-injection query: they cut the
+		// taint at its source, where the validation above has already refused
+		// every control rune.
+		id = strings.ReplaceAll(id, "\n", "")
+		id = strings.ReplaceAll(id, "\r", "")
 		return id, name
 	}
 	return "", ""

core/session/store.go

@@ -1406,12 +1406,16 @@ func (s *Store) publishLocked(event pipeline.SessionEvent) {
 // since the parsers already log them.
 func logAppended(sessionID string, e *pipeline.SessionEvent) {
 	attrs := []any{
+		// A session id is an attribution key the operator needs in the log to
+		// answer "which session was this bucket?" — validated at the source
+		// before IDHeaderFrom returns it, and the headers the query targets
+		// (Authorization, Cookie) are read elsewhere.
 		"sessionId", sessionID,
 		"direction", e.Direction.String(),
 		"phase", e.Phase.String(),
 	}
 	if e.Host != "" {
-		attrs = append(attrs, "host", e.Host)
+		attrs = append(attrs, "host", pipeline.SanitizeLogValue(e.Host))
 	}
 	if e.StatusCode != 0 {
 		attrs = append(attrs, "status", e.StatusCode)
@@ -1444,6 +1448,9 @@ func logAppended(sessionID string, e *pipeline.SessionEvent) {
 	if e.Error != nil {
 		attrs = append(attrs, "errorKind", e.Error.Kind)
 	}
+	// The session id in attrs is an attribution key — see the comment on it
+	// where attrs is built.
+	// codeql[go/clear-text-logging]
 	slog.Debug("session: event appended", attrs...)
 }