From 7f8cbf4c4bf87fe0850e44bd93d664fd54d196be Mon Sep 17 00:00:00 2001 From: n0ctal <4c866w5fn9@privaterelay.appleid.com> Date: Sun, 28 Jun 2026 18:00:55 +0500 Subject: [PATCH] fix(web): tighten database restore body-cap exemption (#5609) --- internal/web/middleware/bodylimit.go | 9 ++++----- internal/web/middleware/bodylimit_test.go | 22 ++++++++++++---------- internal/web/web.go | 6 +++--- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/internal/web/middleware/bodylimit.go b/internal/web/middleware/bodylimit.go index 80fedb7a3..129cd24ce 100644 --- a/internal/web/middleware/bodylimit.go +++ b/internal/web/middleware/bodylimit.go @@ -23,7 +23,7 @@ func MaxBodyBytes(limit int64, skipSuffixes ...string) gin.HandlerFunc { switch c.Request.Method { case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace: default: - if c.Request.Body != nil && !hasSuffix(c.Request.URL.Path, skipSuffixes) { + if c.Request.Body != nil && !hasAnySuffix(c.Request.URL.Path, skipSuffixes) { c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, limit) } } @@ -32,10 +32,9 @@ func MaxBodyBytes(limit int64, skipSuffixes ...string) gin.HandlerFunc { } } -// hasSuffix reports whether path ends in any of the given suffixes. -func hasSuffix(path string, suffixes []string) bool { - for _, s := range suffixes { - if strings.HasSuffix(path, s) { +func hasAnySuffix(path string, suffixes []string) bool { + for _, suffix := range suffixes { + if suffix != "" && strings.HasSuffix(path, suffix) { return true } } diff --git a/internal/web/middleware/bodylimit_test.go b/internal/web/middleware/bodylimit_test.go index 6af392714..b0b6b5aba 100644 --- a/internal/web/middleware/bodylimit_test.go +++ b/internal/web/middleware/bodylimit_test.go @@ -50,7 +50,7 @@ func TestMaxBodyBytes(t *testing.T) { func TestMaxBodyBytesSkipSuffix(t *testing.T) { gin.SetMode(gin.TestMode) - const limit = 16 + const limit = 10 << 20 r := gin.New() r.Use(MaxBodyBytes(limit, "/server/importDB")) @@ -61,20 +61,22 @@ func TestMaxBodyBytesSkipSuffix(t *testing.T) { } c.String(http.StatusOK, "ok") } - r.POST("/server/importDB", read) + r.POST("/prefix/panel/api/server/importDB", read) + r.POST("/prefix/panel/api/server/importDB/other", read) r.POST("/x", read) - // Exempt route reads an over-limit body without error. + large := bytes.Repeat([]byte("x"), limit+1) w := httptest.NewRecorder() - r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/server/importDB", bytes.NewReader(make([]byte, limit*4)))) + r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/prefix/panel/api/server/importDB", bytes.NewReader(large))) if w.Code != http.StatusOK { - t.Errorf("exempt route should pass through over-limit body, got %d", w.Code) + t.Fatalf("restore route should accept an over-limit body, got %d", w.Code) } - // Non-exempt route is still capped. - w = httptest.NewRecorder() - r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/x", bytes.NewReader(make([]byte, limit*4)))) - if w.Code == http.StatusOK { - t.Errorf("non-exempt over-limit POST should not succeed, got 200") + for _, path := range []string{"/x", "/prefix/panel/api/server/importDB/other"} { + w = httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, path, bytes.NewReader(large))) + if w.Code == http.StatusOK { + t.Fatalf("non-exempt path %q accepted an over-limit body", path) + } } } diff --git a/internal/web/web.go b/internal/web/web.go index 9868dfc38..33c400eb1 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -168,9 +168,9 @@ func (s *Server) initRouter() (*gin.Engine, error) { // Cap request bodies on state-changing requests so a stolen session/API // token or a buggy client can't force large allocations or long DB // transactions via bulk create/attach/import endpoints. GET/HEAD/OPTIONS - // carry no body and are left untouched. importDB restores a full SQLite - // backup that legitimately exceeds the cap, so it's exempt. Follow-up: make - // the limit a setting. + // carry no body and are left untouched. Database restore legitimately accepts + // large backups and streams them to disk, so only its exact route suffix is + // exempt. Follow-up: make the limit a setting. const maxRequestBodyBytes = 10 << 20 // 10 MiB engine.Use(middleware.MaxBodyBytes(maxRequestBodyBytes, "/panel/api/server/importDB"))