mirror of
https://github.com/lukaszraczylo/graphql-monitoring-proxy.git
synced 2026-10-22 06:38:13 +00:00
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.
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
"unicode/utf8"
|
||||||
|
|
||||||
"github.com/stretchr/testify/suite"
|
"github.com/stretchr/testify/suite"
|
||||||
)
|
)
|
||||||
@@ -612,3 +613,39 @@ func BenchmarkSanitizeHeaders(b *testing.B) {
|
|||||||
sanitizeHeaders(headers)
|
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))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+11
-4
@@ -23,10 +23,15 @@ type RateLimitConfig struct {
|
|||||||
|
|
||||||
// UnmarshalJSON implements custom JSON unmarshaling for RateLimitConfig
|
// UnmarshalJSON implements custom JSON unmarshaling for RateLimitConfig
|
||||||
func (r *RateLimitConfig) UnmarshalJSON(data []byte) error {
|
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 {
|
type RateLimitConfigTemp struct {
|
||||||
Interval any `json:"interval"`
|
Interval any `json:"interval"`
|
||||||
Req int `json:"req"`
|
Burst int `json:"burst,omitempty"`
|
||||||
|
Endpoints []string `json:"endpoints,omitempty"`
|
||||||
|
Req int `json:"req"`
|
||||||
}
|
}
|
||||||
|
|
||||||
var temp RateLimitConfigTemp
|
var temp RateLimitConfigTemp
|
||||||
@@ -34,8 +39,10 @@ func (r *RateLimitConfig) UnmarshalJSON(data []byte) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set the Req field directly
|
// Set the Req, Burst and Endpoints fields directly
|
||||||
r.Req = temp.Req
|
r.Req = temp.Req
|
||||||
|
r.Burst = temp.Burst
|
||||||
|
r.Endpoints = temp.Endpoints
|
||||||
|
|
||||||
// Handle the Interval field based on its type
|
// Handle the Interval field based on its type
|
||||||
switch v := temp.Interval.(type) {
|
switch v := temp.Interval.(type) {
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/goccy/go-json"
|
"github.com/goccy/go-json"
|
||||||
@@ -275,3 +276,26 @@ func (suite *Tests) Test_RateLimitConfig_UnmarshalJSON() {
|
|||||||
suite.Equal(60*time.Second, config.Interval)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+30
-3
@@ -5,6 +5,7 @@ import (
|
|||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
|
"unicode/utf8"
|
||||||
|
|
||||||
"github.com/goccy/go-json"
|
"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)
|
bodyStr := string(body)
|
||||||
if len(bodyStr) > MaxLogBodySize {
|
if len(body) > MaxLogBodySize {
|
||||||
return bodyStr[:MaxLogBodySize] + TruncatedSuffix
|
// 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
|
// For small non-JSON bodies, do basic string replacement
|
||||||
@@ -116,6 +119,30 @@ func sanitizeForLogging(body []byte, contentType string) string {
|
|||||||
return bodyStr
|
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
|
// redactSensitiveFields recursively redacts sensitive fields in a map
|
||||||
func redactSensitiveFields(data map[string]any, fields []string) {
|
func redactSensitiveFields(data map[string]any, fields []string) {
|
||||||
for key, value := range data {
|
for key, value := range data {
|
||||||
|
|||||||
Reference in New Issue
Block a user