From 51974e52d4302ee6dbebdb36617ed6b06b186ca4 Mon Sep 17 00:00:00 2001 From: Rex Raphael Date: Tue, 29 Sep 2026 16:37:40 -0500 Subject: [PATCH] fix(dashboard): cap the contract envelope before decoding it The contract transport now refuses a request body over 1 MiB with 413 and BAD_REQUEST. You can change the cap with transport.WithMaxBodyBytes, server.WithMaxBodyBytes, or the dashboard's contract_max_body_bytes setting. Zero or less keeps the default, so there's no way to turn it off. Before this, the handler read and decoded the whole envelope before it looked at the intent's Requires predicate, and it had no limit at all. Any principal, read-only ones included, could post an arbitrarily large body to any intent and the server would work through all of it before saying no. We found this while reviewing chronicle's reports.generateCustom, which takes user-supplied text. The Requires check still runs after the decode, because the intent name only exists inside the body. If the declared Content-Length is over the cap, the handler refuses without reading anything. For a chunked body, http.MaxBytesReader stops one byte past the cap. --- extensions/dashboard/config.go | 10 + .../dashboard/contract/server/server.go | 16 +- .../dashboard/contract/server/server_test.go | 15 ++ .../dashboard/contract/transport/http.go | 59 +++++- .../contract/transport/http_limit_test.go | 171 ++++++++++++++++++ extensions/dashboard/extension.go | 4 +- 6 files changed, 266 insertions(+), 9 deletions(-) create mode 100644 extensions/dashboard/contract/transport/http_limit_test.go diff --git a/extensions/dashboard/config.go b/extensions/dashboard/config.go index 42ee1878..a1490f0b 100644 --- a/extensions/dashboard/config.go +++ b/extensions/dashboard/config.go @@ -104,6 +104,9 @@ type Config struct { // set to false during a rollout window where clients have not yet // adopted CSRF tokens or the idempotency-key contract. EnableContractSecurity bool `json:"enable_contract_security" yaml:"enable_contract_security"` + // ContractMaxBodyBytes caps the contract envelope a client may POST. + // Zero or less means transport.DefaultMaxBodyBytes (1 MiB). + ContractMaxBodyBytes int64 `json:"contract_max_body_bytes" yaml:"contract_max_body_bytes"` // Authentication EnableAuth bool `json:"enable_auth" yaml:"enable_auth"` // enable auth support @@ -387,6 +390,13 @@ func WithContractSecurity(enabled bool) ConfigOption { return func(c *Config) { c.EnableContractSecurity = enabled } } +// WithContractMaxBodyBytes caps the contract envelope a client may POST. +// Larger bodies are refused with 413 before they are decoded. Zero or less +// keeps the 1 MiB default. +func WithContractMaxBodyBytes(n int64) ConfigOption { + return func(c *Config) { c.ContractMaxBodyBytes = n } +} + // WithTheme sets the UI theme (light, dark, auto). func WithTheme(theme string) ConfigOption { return func(c *Config) { c.Theme = theme } diff --git a/extensions/dashboard/contract/server/server.go b/extensions/dashboard/contract/server/server.go index 018bed18..154fa788 100644 --- a/extensions/dashboard/contract/server/server.go +++ b/extensions/dashboard/contract/server/server.go @@ -34,6 +34,7 @@ type Server struct { reg contract.Registry dispatch http.Handler prefix string + handler []transport.HandlerOption } // Option configures a Server. @@ -50,6 +51,14 @@ func WithPrefix(p string) Option { } } +// WithMaxBodyBytes caps the dispatch request envelope at n bytes. The +// default is transport.DefaultMaxBodyBytes; n <= 0 keeps it. +func WithMaxBodyBytes(n int64) Option { + return func(s *Server) { + s.handler = append(s.handler, transport.WithMaxBodyBytes(n)) + } +} + // New returns a Server configured to serve the registry + dispatcher // passed in. The supplied audit emitter is plumbed through to the // dispatch handler; pass contract.NoopAuditEmitter{} when not needed. @@ -61,13 +70,14 @@ func New( opts ...Option, ) *Server { s := &Server{ - reg: reg, - dispatch: transport.NewHandler(reg, wreg, disp, audit), - prefix: DefaultPrefix, + reg: reg, + prefix: DefaultPrefix, } for _, o := range opts { o(s) } + + s.dispatch = transport.NewHandler(reg, wreg, disp, audit, s.handler...) return s } diff --git a/extensions/dashboard/contract/server/server_test.go b/extensions/dashboard/contract/server/server_test.go index c0fc719c..66ca517b 100644 --- a/extensions/dashboard/contract/server/server_test.go +++ b/extensions/dashboard/contract/server/server_test.go @@ -168,3 +168,18 @@ func TestRoundTrip_HostToUpstream(t *testing.T) { t.Errorf("round trip data = %+v", got) } } + +func TestServer_WithMaxBodyBytesReachesDispatch(t *testing.T) { + s := New(contract.NewRegistry(), contract.NewWardenRegistry(), nil, contract.NoopAuditEmitter{}, WithMaxBodyBytes(64)) + body, _ := json.Marshal(contract.Request{ + Envelope: "v1", Kind: contract.KindQuery, + Contributor: "things", Intent: "things.list", IntentVersion: 1, + Payload: json.RawMessage(`"` + strings.Repeat("a", 64) + `"`), + }) + req := httptest.NewRequest(http.MethodPost, s.DispatchPath(), bytes.NewReader(body)) + w := httptest.NewRecorder() + s.ServeHTTP(w, req) + if w.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413; body=%s", w.Code, w.Body) + } +} diff --git a/extensions/dashboard/contract/transport/http.go b/extensions/dashboard/contract/transport/http.go index 62e67619..2d8a4589 100644 --- a/extensions/dashboard/contract/transport/http.go +++ b/extensions/dashboard/contract/transport/http.go @@ -4,7 +4,9 @@ package transport import ( "context" "encoding/json" + "errors" "net/http" + "strconv" "time" dashauth "github.com/xraph/forge/extensions/dashboard/auth" @@ -31,23 +33,49 @@ func (NilDispatcher) Dispatch(_ context.Context, _ contract.Request, _ contract. // supportedEnvelopes is the set this slice's handler understands. var supportedEnvelopes = map[string]bool{"v1": true} +// DefaultMaxBodyBytes caps the request envelope when no WithMaxBodyBytes +// option is given. The whole envelope is decoded before the intent's +// Requires predicate can run, so this cap is what bounds the work an +// unauthorized caller can make the server do. +const DefaultMaxBodyBytes int64 = 1 << 20 + +// HandlerOption configures the handler returned by NewHandler. +type HandlerOption func(*handler) + +// WithMaxBodyBytes sets the largest request envelope the handler will read. +// A larger body is refused with 413 and CodeBadRequest. n <= 0 keeps +// DefaultMaxBodyBytes; there is no way to switch the cap off. +func WithMaxBodyBytes(n int64) HandlerOption { + return func(h *handler) { + if n > 0 { + h.maxBody = n + } + } +} + // NewHandler returns the POST /api/dashboard/{envelope} handler. -func NewHandler(reg contract.Registry, wreg contract.WardenRegistry, disp Dispatcher, audit contract.AuditEmitter) http.Handler { +func NewHandler(reg contract.Registry, wreg contract.WardenRegistry, disp Dispatcher, audit contract.AuditEmitter, opts ...HandlerOption) http.Handler { if disp == nil { disp = NilDispatcher{} } if audit == nil { audit = contract.NoopAuditEmitter{} } - return &handler{reg: reg, wreg: wreg, disp: disp, audit: audit} + + h := &handler{reg: reg, wreg: wreg, disp: disp, audit: audit, maxBody: DefaultMaxBodyBytes} + for _, o := range opts { + o(h) + } + + return h } // NewHandlerWithCSRF is NewHandler plus a CSRFManager for command validation. // When mgr is non-nil, command envelopes whose CSRF token does not validate // return CodeUnauthenticated. Pass nil to skip CSRF (preserves the slice-(a) // behaviour for tests and rollout opt-out). -func NewHandlerWithCSRF(reg contract.Registry, wreg contract.WardenRegistry, disp Dispatcher, audit contract.AuditEmitter, mgr *security.CSRFManager) http.Handler { - h := NewHandler(reg, wreg, disp, audit).(*handler) +func NewHandlerWithCSRF(reg contract.Registry, wreg contract.WardenRegistry, disp Dispatcher, audit contract.AuditEmitter, mgr *security.CSRFManager, opts ...HandlerOption) http.Handler { + h := NewHandler(reg, wreg, disp, audit, opts...).(*handler) h.csrfMgr = mgr return h } @@ -58,6 +86,7 @@ type handler struct { disp Dispatcher audit contract.AuditEmitter csrfMgr *security.CSRFManager // optional; nil disables CSRF validation + maxBody int64 // envelope size cap; always > 0 } func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -66,8 +95,21 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } defer r.Body.Close() + // The intent name lives inside the envelope, so its Requires predicate + // cannot run until the body is decoded. Cap the body first: any caller, + // authorized or not, can otherwise make us read and decode without bound. + // A declared length over the cap is refused without reading a byte. + if r.ContentLength > h.maxBody { + writeBodyTooLarge(w, h.maxBody) + return + } var req contract.Request - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, h.maxBody)).Decode(&req); err != nil { + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + writeBodyTooLarge(w, h.maxBody) + return + } writeError(w, http.StatusBadRequest, &contract.Error{Code: contract.CodeBadRequest, Message: "invalid JSON: " + err.Error()}) return } @@ -234,6 +276,13 @@ func writeOK(w http.ResponseWriter, r contract.Response) { _ = json.NewEncoder(w).Encode(r) } +func writeBodyTooLarge(w http.ResponseWriter, limit int64) { + writeError(w, http.StatusRequestEntityTooLarge, &contract.Error{ + Code: contract.CodeBadRequest, + Message: "request body exceeds " + strconv.FormatInt(limit, 10) + " bytes", + }) +} + func writeError(w http.ResponseWriter, status int, e *contract.Error) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) diff --git a/extensions/dashboard/contract/transport/http_limit_test.go b/extensions/dashboard/contract/transport/http_limit_test.go new file mode 100644 index 00000000..2ddffee8 --- /dev/null +++ b/extensions/dashboard/contract/transport/http_limit_test.go @@ -0,0 +1,171 @@ +// http_limit_test.go +package transport + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/xraph/forge/extensions/dashboard/contract" +) + +// countingReader records how many bytes the handler pulled off the wire, so +// a test can tell "refused after reading 1 KiB" from "refused after reading +// all 4 MiB". +type countingReader struct { + r io.Reader + n int64 +} + +func (c *countingReader) Read(p []byte) (int, error) { + n, err := c.r.Read(p) + c.n += int64(n) + return n, err +} + +// envelopeOfSize returns a valid users.list query envelope that is exactly +// size bytes long, padded through its payload. +func envelopeOfSize(t *testing.T, size int64) []byte { + t.Helper() + mk := func(pad int64) []byte { + b, err := json.Marshal(contract.Request{ + Envelope: "v1", Kind: contract.KindQuery, Contributor: "users", Intent: "users.list", IntentVersion: 1, + Payload: json.RawMessage(`"` + strings.Repeat("a", int(pad)) + `"`), + }) + if err != nil { + t.Fatal(err) + } + return b + } + base := int64(len(mk(0))) + if size < base { + t.Fatalf("size %d is below the minimum envelope size %d", size, base) + } + b := mk(size - base) + if int64(len(b)) != size { + t.Fatalf("envelope is %d bytes, want %d", len(b), size) + } + return b +} + +// postBody sends body with no declared length, the way a chunked upload +// arrives, so the handler cannot refuse it from the header alone. +func postBody(h http.Handler, body []byte) (*httptest.ResponseRecorder, *countingReader) { + cr := &countingReader{r: bytes.NewReader(body)} + req := httptest.NewRequest(http.MethodPost, "/api/dashboard/v1", cr) + req.ContentLength = -1 + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + return w, cr +} + +func assertTooLarge(t *testing.T, w *httptest.ResponseRecorder, disp *stubDispatcher) { + t.Helper() + if w.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413; body=%.200s", w.Code, w.Body) + } + var resp contract.ErrorResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode error response: %v", err) + } + if resp.Error == nil || resp.Error.Code != contract.CodeBadRequest { + t.Errorf("error = %+v, want code %s", resp.Error, contract.CodeBadRequest) + } + if disp.called != "" { + t.Errorf("dispatcher ran for an oversized body: %s", disp.called) + } +} + +func TestHandler_RefusesOversizedBodyWithoutReadingIt(t *testing.T) { + reg, wreg := setupRegistry(t) + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}) + + body := envelopeOfSize(t, 4*DefaultMaxBodyBytes) + w, cr := postBody(h, body) + + assertTooLarge(t, w, disp) + // http.MaxBytesReader stops one byte past the cap. Anything more means + // the handler kept reading after it knew the answer. + if cr.n > DefaultMaxBodyBytes+1 { + t.Errorf("handler read %d of %d bytes; want at most %d", cr.n, len(body), DefaultMaxBodyBytes+1) + } +} + +func TestHandler_RefusesOversizedDeclaredLengthBeforeReading(t *testing.T) { + reg, wreg := setupRegistry(t) + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}, WithMaxBodyBytes(1024)) + + body := envelopeOfSize(t, 2048) + cr := &countingReader{r: bytes.NewReader(body)} + req := httptest.NewRequest(http.MethodPost, "/api/dashboard/v1", cr) + req.ContentLength = int64(len(body)) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + assertTooLarge(t, w, disp) + if cr.n != 0 { + t.Errorf("handler read %d bytes of a body whose declared length was already over the cap", cr.n) + } +} + +func TestHandler_AcceptsBodyAtDefaultLimit(t *testing.T) { + reg, wreg := setupRegistry(t) + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}) + + w, _ := postBody(h, envelopeOfSize(t, DefaultMaxBodyBytes)) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%.200s", w.Code, w.Body) + } + if disp.called != "query:users.list" { + t.Errorf("dispatcher not called: %q", disp.called) + } +} + +func TestHandler_HonoursConfiguredLimit(t *testing.T) { + const limit = 4096 + reg, wreg := setupRegistry(t) + + t.Run("at limit", func(t *testing.T) { + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}, WithMaxBodyBytes(limit)) + w, _ := postBody(h, envelopeOfSize(t, limit)) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%.200s", w.Code, w.Body) + } + }) + + // One byte over a configured cap that sits far below the default, so a + // handler that ignored the option would accept it. + t.Run("one byte over", func(t *testing.T) { + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}, WithMaxBodyBytes(limit)) + w, cr := postBody(h, envelopeOfSize(t, limit+1)) + assertTooLarge(t, w, disp) + if cr.n > limit+1 { + t.Errorf("handler read %d bytes; want at most %d", cr.n, limit+1) + } + }) + + t.Run("raised above default", func(t *testing.T) { + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}, WithMaxBodyBytes(2*DefaultMaxBodyBytes)) + w, _ := postBody(h, envelopeOfSize(t, DefaultMaxBodyBytes+1)) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%.200s", w.Code, w.Body) + } + }) + + t.Run("non-positive keeps default", func(t *testing.T) { + disp := &stubDispatcher{response: json.RawMessage(`{}`)} + h := NewHandler(reg, wreg, disp, contract.NoopAuditEmitter{}, WithMaxBodyBytes(0)) + w, _ := postBody(h, envelopeOfSize(t, DefaultMaxBodyBytes+1)) + assertTooLarge(t, w, disp) + }) +} diff --git a/extensions/dashboard/extension.go b/extensions/dashboard/extension.go index 50ed086d..a75f102b 100644 --- a/extensions/dashboard/extension.go +++ b/extensions/dashboard/extension.go @@ -1856,7 +1856,9 @@ func (e *Extension) handleContractPOST() http.HandlerFunc { if e.config.EnableContractSecurity && e.csrfMgr != nil { mgr = e.csrfMgr } - h := transport.NewHandlerWithCSRF(e.contractRegistry, e.wardenRegistry, e.dispatcher, e.auditEmitter, mgr) + + h := transport.NewHandlerWithCSRF(e.contractRegistry, e.wardenRegistry, e.dispatcher, e.auditEmitter, mgr, + transport.WithMaxBodyBytes(e.config.ContractMaxBodyBytes)) return h.ServeHTTP }