diff --git a/coderd/httpmw/loggermw/logger.go b/coderd/httpmw/loggermw/logger.go index 8f21f9aa32..37e15b3bfc 100644 --- a/coderd/httpmw/loggermw/logger.go +++ b/coderd/httpmw/loggermw/logger.go @@ -4,6 +4,9 @@ import ( "context" "fmt" "net/http" + "net/url" + "strconv" + "strings" "sync" "time" @@ -15,6 +18,59 @@ import ( "github.com/coder/coder/v2/coderd/tracing" ) +var ( + safeParams = []string{"page", "limit", "offset"} + countParams = []string{"ids", "template_ids"} +) + +func safeQueryParams(params url.Values) []slog.Field { + if len(params) == 0 { + return nil + } + + fields := make([]slog.Field, 0, len(params)) + for key, values := range params { + // Check if this parameter should be included + for _, pattern := range safeParams { + if strings.EqualFold(key, pattern) { + // Prepend query parameters in the log line to ensure we don't have issues with collisions + // in case any other internal logging fields already log fields with similar names + fieldName := "query_" + key + + // Log the actual values for non-sensitive parameters + if len(values) == 1 { + fields = append(fields, slog.F(fieldName, values[0])) + continue + } + fields = append(fields, slog.F(fieldName, values)) + } + } + // Some query params we just want to log the count of the params length + for _, pattern := range countParams { + if !strings.EqualFold(key, pattern) { + continue + } + count := 0 + + // Prepend query parameters in the log line to ensure we don't have issues with collisions + // in case any other internal logging fields already log fields with similar names + fieldName := "query_" + key + + // Count comma-separated values for CSV format + for _, v := range values { + if strings.Contains(v, ",") { + count += len(strings.Split(v, ",")) + continue + } + count++ + } + // For logging we always want strings + fields = append(fields, slog.F(fieldName+"_count", strconv.Itoa(count))) + } + } + return fields +} + func Logger(log slog.Logger) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -39,6 +95,11 @@ func Logger(log slog.Logger) func(next http.Handler) http.Handler { slog.F("start", start), ) + // Add safe query parameters to the log + if queryFields := safeQueryParams(r.URL.Query()); len(queryFields) > 0 { + httplog = httplog.With(queryFields...) + } + logContext := NewRequestLogger(httplog, r.Method, start) ctx := WithRequestLogger(r.Context(), logContext) diff --git a/coderd/httpmw/loggermw/logger_internal_test.go b/coderd/httpmw/loggermw/logger_internal_test.go index f372c665fd..bf09046424 100644 --- a/coderd/httpmw/loggermw/logger_internal_test.go +++ b/coderd/httpmw/loggermw/logger_internal_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "net/url" "slices" "strings" "sync" @@ -292,6 +293,76 @@ func TestRequestLogger_RouteParamsLogging(t *testing.T) { } } +func TestSafeQueryParams(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + params url.Values + expected map[string]interface{} + }{ + { + name: "safe parameters", + params: url.Values{ + "page": []string{"1"}, + "limit": []string{"10"}, + "filter": []string{"active"}, + "sort": []string{"name"}, + "offset": []string{"2"}, + "ids": []string{"some-id,another-id", "second-param"}, + "template_ids": []string{"some-id,another-id", "second-param"}, + }, + expected: map[string]interface{}{ + "query_page": "1", + "query_limit": "10", + "query_offset": "2", + "query_ids_count": "3", + "query_template_ids_count": "3", + }, + }, + { + name: "unknown/sensitive parameters", + params: url.Values{ + "token": []string{"secret-token"}, + "api_key": []string{"secret-key"}, + "coder_signed_app_token": []string{"jwt-token"}, + "coder_application_connect_api_key": []string{"encrypted-key"}, + "client_secret": []string{"oauth-secret"}, + "code": []string{"auth-code"}, + }, + expected: map[string]interface{}{}, + }, + { + name: "mixed parameters", + params: url.Values{ + "page": []string{"1"}, + "token": []string{"secret"}, + "filter": []string{"active"}, + }, + expected: map[string]interface{}{ + "query_page": "1", + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fields := safeQueryParams(tt.params) + + // Convert fields to map for easier comparison + result := make(map[string]interface{}) + for _, field := range fields { + result[field.Name] = field.Value + } + + require.Equal(t, tt.expected, result) + }) + } +} + type fakeSink struct { entries []slog.SinkEntry newEntries chan slog.SinkEntry