From cea69a4117bb55ad0d386922829fc23654412eca Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 31 Oct 2026 09:09:06 +0000 Subject: [PATCH] fix(api): accept config envelopes only on requests that carry a body The envelope middleware buffered and decoded any request carrying its headers, GETs included, which the router-wide body cap skips and which read-only tokens may call; its raw read had no cap of its own. Refuse an envelope on safe methods and read at most the decoded-size limit. --- internal/web/middleware/config_envelope.go | 14 ++++++++- .../web/middleware/config_envelope_test.go | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/internal/web/middleware/config_envelope.go b/internal/web/middleware/config_envelope.go index 2f2acaaf4..163a64d4c 100644 --- a/internal/web/middleware/config_envelope.go +++ b/internal/web/middleware/config_envelope.go @@ -39,11 +39,23 @@ func ConfigEnvelopeMiddleware() gin.HandlerFunc { return } - raw, err := io.ReadAll(c.Request.Body) + // Safe methods carry no config: an envelope there is never legitimate, and read-only + // (monitor) callers must not reach the decoder. + switch c.Request.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace: + c.AbortWithStatus(http.StatusBadRequest) + return + } + + raw, err := io.ReadAll(io.LimitReader(c.Request.Body, maxDecodedConfigBytes+1)) if err != nil { c.AbortWithStatus(http.StatusBadRequest) return } + if len(raw) > maxDecodedConfigBytes { + c.AbortWithStatus(http.StatusRequestEntityTooLarge) + return + } _ = c.Request.Body.Close() if enc == wirecodec.EncodingZstd { diff --git a/internal/web/middleware/config_envelope_test.go b/internal/web/middleware/config_envelope_test.go index 023fd6e66..4f081eced 100644 --- a/internal/web/middleware/config_envelope_test.go +++ b/internal/web/middleware/config_envelope_test.go @@ -80,3 +80,34 @@ func TestConfigEnvelope_PlainPassesThrough(t *testing.T) { t.Fatalf("a request with no envelope headers must pass through unchanged: code=%d body=%q", w.Code, w.Body.String()) } } + +// A read-only (monitor) token reaches GET routes only; an envelope never rides on one, so +// it must not make the panel buffer and decode a body there. +func TestConfigEnvelope_RefusesAnEnvelopeOnASafeMethod(t *testing.T) { + called := false + engine := envelopeTestEngine(t, nil) + engine.GET("/status", func(c *gin.Context) { called = true }) + body := bytes.Repeat([]byte("A"), 1<<20) + req := httptest.NewRequest(http.MethodGet, "/status", bytes.NewReader(wirecodec.Compress(body))) + req.Header.Set("Content-Encoding", wirecodec.EncodingZstd) + w := httptest.NewRecorder() + engine.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest || called { + t.Fatalf("GET with an envelope = %d (handler called %v), want 400 before the handler", w.Code, called) + } +} + +// The router-wide body cap does not cover every route, so the envelope reads at most +// what it would accept once decoded. +func TestConfigEnvelope_CapsTheRawBody(t *testing.T) { + called := false + engine := envelopeTestEngine(t, func() { called = true }) + body := bytes.Repeat([]byte("A"), maxDecodedConfigBytes+1) + req := httptest.NewRequest(http.MethodPost, "/echo", bytes.NewReader(body)) + req.Header.Set(wirecodec.HashHeader, wirecodec.Sha256Hex(body)) + w := httptest.NewRecorder() + engine.ServeHTTP(w, req) + if w.Code != http.StatusRequestEntityTooLarge || called { + t.Fatalf("oversized envelope body = %d (handler called %v), want 413", w.Code, called) + } +}