From fc28c6568119c9b72403558c19f6f8b77da70374 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 13 Aug 2026 05:55:36 +0100 Subject: [PATCH] fix(proxy): keep Burst/Endpoints in ratelimit unmarshal; redact large truncated log bodies - ratelimit: custom UnmarshalJSON temp struct only carried interval/req, silently dropping the configured 'burst' and 'endpoints' fields. Burst is consumed at checkRateLimit (burst allowance) but could never be set via config file. Carry all fields in the temp struct. - sanitization: a large non-parsable body was truncated before redaction, potentially leaking a secret in the first MaxLogBodySize bytes, and the truncation could split a multi-byte UTF-8 rune. Redact the retained prefix and truncate on a rune boundary (sanitizeTruncatedBody). - tests: TestRateLimitConfigUnmarshalKeepsBurstAndEndpoints, TestSanitizeTruncatedBodyRedactsAndKeepsUTF8, TestTruncateUTF8RuneBoundary. --- proxy_logging_security_test.go | 37 ++++++++++++++++++++++++++++++++++ ratelimit.go | 15 ++++++++++---- ratelimit_test.go | 24 ++++++++++++++++++++++ sanitization.go | 33 +++++++++++++++++++++++++++--- 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/proxy_logging_security_test.go b/proxy_logging_security_test.go index 76dcba4..657acd7 100644 --- a/proxy_logging_security_test.go +++ b/proxy_logging_security_test.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" "testing" + "unicode/utf8" "github.com/stretchr/testify/suite" ) @@ -612,3 +613,39 @@ func BenchmarkSanitizeHeaders(b *testing.B) { sanitizeHeaders(headers) } } + +func (suite *ProxyLoggingSecurityTestSuite) TestSanitizeTruncatedBodyRedactsAndKeepsUTF8() { + // A large body whose JSON parse fails (malformed JSON) must still redact + // sensitive fields in the retained prefix, and the truncation must not + // cut a multi-byte UTF-8 codepoint. + secret := "hunter2-S3cret" + big := strings.Repeat("a", MaxLogBodySize+10) + // Unterminated string -> JSON decode fails -> falls through to truncation. + malformed := `{"password": "` + secret + `"` + big + + result := sanitizeForLogging([]byte(malformed), "application/json") + if strings.Contains(result, secret) { + suite.T().Errorf("truncated body leaked secret: %q", result) + } + if !strings.Contains(result, RedactedPlaceholder) { + suite.T().Errorf("expected redaction placeholder in %q", result) + } + if !utf8.ValidString(result) { + suite.T().Errorf("result is not valid UTF-8: %q", result) + } + if !strings.HasSuffix(result, TruncatedSuffix) { + suite.T().Errorf("expected truncated suffix in %q", result) + } +} + +func (suite *ProxyLoggingSecurityTestSuite) TestTruncateUTF8RuneBoundary() { + // A multi-byte rune straddling the byte cutoff must be backed off, not split. + orig := strings.Repeat("a", MaxLogBodySize-1) + "界" + out := truncateUTF8(orig, MaxLogBodySize) + if !utf8.ValidString(out) { + suite.T().Fatalf("truncateUTF8 produced invalid UTF-8: %x", out) + } + if len(out) != MaxLogBodySize-1 { + suite.T().Fatalf("expected backoff to %d bytes, got %d", MaxLogBodySize-1, len(out)) + } +} diff --git a/ratelimit.go b/ratelimit.go index a924a9a..543fb85 100644 --- a/ratelimit.go +++ b/ratelimit.go @@ -23,10 +23,15 @@ type RateLimitConfig struct { // UnmarshalJSON implements custom JSON unmarshaling for RateLimitConfig func (r *RateLimitConfig) UnmarshalJSON(data []byte) error { - // Use a temporary struct to unmarshal the JSON data + // Use a temporary struct to unmarshal the JSON data. + // It must carry every configurable field: Burst (used by the burst + // allowance at the top of checkRateLimit) and Endpoints would otherwise be + // silently dropped from the loaded config and left at their zero values. type RateLimitConfigTemp struct { - Interval any `json:"interval"` - Req int `json:"req"` + Interval any `json:"interval"` + Burst int `json:"burst,omitempty"` + Endpoints []string `json:"endpoints,omitempty"` + Req int `json:"req"` } var temp RateLimitConfigTemp @@ -34,8 +39,10 @@ func (r *RateLimitConfig) UnmarshalJSON(data []byte) error { return err } - // Set the Req field directly + // Set the Req, Burst and Endpoints fields directly r.Req = temp.Req + r.Burst = temp.Burst + r.Endpoints = temp.Endpoints // Handle the Interval field based on its type switch v := temp.Interval.(type) { diff --git a/ratelimit_test.go b/ratelimit_test.go index 0f8ee84..22b707b 100644 --- a/ratelimit_test.go +++ b/ratelimit_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "testing" "time" "github.com/goccy/go-json" @@ -275,3 +276,26 @@ func (suite *Tests) Test_RateLimitConfig_UnmarshalJSON() { suite.Equal(60*time.Second, config.Interval) }) } + +func TestRateLimitConfigUnmarshalKeepsBurstAndEndpoints(t *testing.T) { + // Regression: the custom UnmarshalJSON temp struct used to carry only + // interval/req, silently dropping the configured burst and endpoints + // fields. A config with "burst" or "endpoints" must keep them. + raw := `{"interval":"minute","req":100,"burst":250,"endpoints":["/v1/graphql"]}` + var rc RateLimitConfig + if err := json.Unmarshal([]byte(raw), &rc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if rc.Interval != time.Minute { + t.Errorf("interval = %v, want %v", rc.Interval, time.Minute) + } + if rc.Req != 100 { + t.Errorf("req = %d, want 100", rc.Req) + } + if rc.Burst != 250 { + t.Errorf("burst = %d, want 250", rc.Burst) + } + if len(rc.Endpoints) != 1 || rc.Endpoints[0] != "/v1/graphql" { + t.Errorf("endpoints = %v, want [\"/v1/graphql\"]", rc.Endpoints) + } +} diff --git a/sanitization.go b/sanitization.go index 6e0bedd..bd71019 100644 --- a/sanitization.go +++ b/sanitization.go @@ -5,6 +5,7 @@ import ( "regexp" "strings" "sync" + "unicode/utf8" "github.com/goccy/go-json" ) @@ -102,10 +103,12 @@ func sanitizeForLogging(body []byte, contentType string) string { } } - // For non-JSON or failed parsing, truncate to prevent logging large bodies bodyStr := string(body) - if len(bodyStr) > MaxLogBodySize { - return bodyStr[:MaxLogBodySize] + TruncatedSuffix + if len(body) > MaxLogBodySize { + // Redact the retained prefix AND keep it on a UTF-8 rune boundary, so + // truncating a large (non-parsable) body never exposes an unredacted + // secret in the logged window nor cuts a multi-byte codepoint. + return sanitizeTruncatedBody(body) } // For small non-JSON bodies, do basic string replacement @@ -116,6 +119,30 @@ func sanitizeForLogging(body []byte, contentType string) string { return bodyStr } +// sanitizeTruncatedBody redacts sensitive fields within the first +// MaxLogBodySize bytes of a large body, then truncates on a UTF-8 rune +// boundary, so a truncated log line is both masked and decodable. +func sanitizeTruncatedBody(body []byte) string { + prefix := string(body[:MaxLogBodySize]) + for _, field := range sensitiveFieldPatterns { + prefix = redactPatternInString(prefix, field) + } + return truncateUTF8(prefix, MaxLogBodySize) + TruncatedSuffix +} + +// truncateUTF8 truncates s to at most maxBytes on a UTF-8 rune boundary. +func truncateUTF8(s string, maxBytes int) string { + if len(s) <= maxBytes { + return s + } + s = s[:maxBytes] + for len(s) > 0 && !utf8.ValidString(s) { + // Back off one byte at a time up to the lead of a partial rune. + s = s[:len(s)-1] + } + return s +} + // redactSensitiveFields recursively redacts sensitive fields in a map func redactSensitiveFields(data map[string]any, fields []string) { for key, value := range data {