From 69d33753955d0eb52cd6d5071f503768542498fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 00:49:15 +0000 Subject: [PATCH 1/2] Initial plan From 5b9272bc7c21d5bca0f358363cf5b9e385287a0d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 00:58:50 +0000 Subject: [PATCH 2/2] fix(openapi): address review feedback from thread 3844286430 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add configurable max body size limit (default 1 MiB) via http.MaxBytesReader to prevent DoS from arbitrarily large request bodies - Use validateJSONValue() for request body validation to handle non-object root schemas (primitives, arrays) that were previously silently skipped - Only register /openapi.yaml endpoint when source spec is YAML; JSON sources already served via /openapi.json - Sort supportedContentTypes() output for deterministic error messages - Remove /api/v1 from plugin schema DefaultConfig to match factory (empty) default - Add server→router mapping in wireOpenAPIRoutes for consistent router discovery when openapi module depends on http.server instead of http.router directly - Tests: add TestOpenAPIModule_JSONSourceNoYAMLEndpoint and TestOpenAPIModule_MaxBodySize Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- module/openapi.go | 71 +++++++++++++++++++++++++-------------- module/openapi_test.go | 64 +++++++++++++++++++++++++++++++++++ plugins/openapi/plugin.go | 44 ++++++++++++++++++++---- 3 files changed, 147 insertions(+), 32 deletions(-) diff --git a/module/openapi.go b/module/openapi.go index 9698a0cc..be3ef561 100644 --- a/module/openapi.go +++ b/module/openapi.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "html" "io" @@ -12,6 +13,7 @@ import ( "net/http" "os" "regexp" + "sort" "strconv" "strings" @@ -33,13 +35,18 @@ type OpenAPISwaggerUIConfig struct { // OpenAPIConfig holds the full configuration for an OpenAPI module. type OpenAPIConfig struct { - SpecFile string `yaml:"spec_file" json:"spec_file"` - BasePath string `yaml:"base_path" json:"base_path"` - Validation OpenAPIValidationConfig `yaml:"validation" json:"validation"` - SwaggerUI OpenAPISwaggerUIConfig `yaml:"swagger_ui" json:"swagger_ui"` - RouterName string `yaml:"router" json:"router"` // optional: explicit router to attach to + SpecFile string `yaml:"spec_file" json:"spec_file"` + BasePath string `yaml:"base_path" json:"base_path"` + Validation OpenAPIValidationConfig `yaml:"validation" json:"validation"` + SwaggerUI OpenAPISwaggerUIConfig `yaml:"swagger_ui" json:"swagger_ui"` + RouterName string `yaml:"router" json:"router"` // optional: explicit router to attach to + MaxBodyBytes int64 `yaml:"max_body_bytes" json:"max_body_bytes"` // max request body size (bytes); 0 = use default } +// defaultMaxBodyBytes is the default request body size limit (1 MiB) applied +// when Validation.Request is enabled and MaxBodyBytes is not explicitly set. +const defaultMaxBodyBytes int64 = 1 << 20 // 1 MiB + // ---- Minimal OpenAPI v3 structs (parsed from YAML/JSON) ---- // openAPISpec is a minimal representation of an OpenAPI 3.x specification. @@ -221,7 +228,7 @@ func (m *OpenAPIModule) RegisterRoutes(router HTTPRouter) { } } - // Serve raw spec at /openapi.json and /openapi.yaml + // Serve raw spec at /openapi.json and (when source is YAML) /openapi.yaml if len(m.specBytes) > 0 { // Cache the JSON representation once per registration. if m.specJSON == nil { @@ -233,21 +240,21 @@ func (m *OpenAPIModule) RegisterRoutes(router HTTPRouter) { } specPathJSON := basePath + "/openapi.json" - specPathYAML := basePath + "/openapi.yaml" // JSON endpoint: serve re-serialised spec as JSON. router.AddRoute(http.MethodGet, specPathJSON, &openAPISpecHandler{specJSON: m.specJSON}) - // YAML endpoint: serve the original spec bytes with a content-type that - // matches the source format. JSON source files are served as application/json; - // YAML source files are served as application/yaml. - rawContentType := "application/yaml" - if trimmed := strings.TrimSpace(string(m.specBytes)); len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '[') { - rawContentType = "application/json" + // YAML endpoint: only register /openapi.yaml when the source spec is YAML. + // When the source is JSON, clients can use /openapi.json instead. + trimmed := strings.TrimSpace(string(m.specBytes)) + isJSONSource := len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '[') + if !isJSONSource { + specPathYAML := basePath + "/openapi.yaml" + router.AddRoute(http.MethodGet, specPathYAML, &openAPIRawSpecHandler{specBytes: m.specBytes, contentType: "application/yaml"}) + m.logger.Debug("OpenAPI spec endpoint registered", "module", m.name, "paths", []string{specPathJSON, specPathYAML}) + } else { + m.logger.Debug("OpenAPI spec endpoint registered", "module", m.name, "paths", []string{specPathJSON}) } - router.AddRoute(http.MethodGet, specPathYAML, &openAPIRawSpecHandler{specBytes: m.specBytes, contentType: rawContentType}) - - m.logger.Debug("OpenAPI spec endpoint registered", "module", m.name, "paths", []string{specPathJSON, specPathYAML}) } // Serve Swagger UI @@ -365,16 +372,29 @@ func (h *openAPIRouteHandler) validate(r *http.Request) []string { ct, supportedContentTypes(h.op.RequestBody.Content))) } + // Enforce a max body size to prevent DoS via arbitrarily large payloads. + // The limit is configurable via OpenAPIConfig.MaxBodyBytes; default is 1 MiB. + maxBytes := h.module.cfg.MaxBodyBytes + if maxBytes <= 0 { + maxBytes = defaultMaxBodyBytes + } + r.Body = http.MaxBytesReader(nil, r.Body, maxBytes) + // Read the body once so we can both check for presence (when required) // and validate against the schema. Restore it afterwards for downstream handlers. bodyBytes, readErr := io.ReadAll(r.Body) if readErr != nil { - h.module.logger.Error("failed to read request body for validation", - "module", h.module.name, - "path", h.specPath, - "error", readErr, - ) - errs = append(errs, "failed to read request body") + var maxBytesErr *http.MaxBytesError + if errors.As(readErr, &maxBytesErr) { + errs = append(errs, fmt.Sprintf("request body exceeds maximum allowed size of %d bytes", maxBytes)) + } else { + h.module.logger.Error("failed to read request body for validation", + "module", h.module.name, + "path", h.specPath, + "error", readErr, + ) + errs = append(errs, "failed to read request body") + } } else { // Always restore body for downstream handlers using the original byte slice // to avoid a bytes→string→bytes conversion that could corrupt non-UTF-8 payloads. @@ -386,7 +406,7 @@ func (h *openAPIRouteHandler) validate(r *http.Request) []string { var bodyData any if jsonErr := json.Unmarshal(bodyBytes, &bodyData); jsonErr != nil { errs = append(errs, fmt.Sprintf("request body contains invalid JSON: %v", jsonErr)) - } else if bodyErrs := validateJSONBody(bodyData, mediaType.Schema); len(bodyErrs) > 0 { + } else if bodyErrs := validateJSONValue(bodyData, "body", mediaType.Schema); len(bodyErrs) > 0 { errs = append(errs, bodyErrs...) } } @@ -728,12 +748,13 @@ func htmlEscape(s string) string { return html.EscapeString(s) } -// supportedContentTypes returns a comma-joined list of content types defined -// in the requestBody.content map, used in validation error messages. +// supportedContentTypes returns a sorted, comma-joined list of content types +// defined in the requestBody.content map, used in validation error messages. func supportedContentTypes(content map[string]openAPIMediaType) string { types := make([]string, 0, len(content)) for ct := range content { types = append(types, ct) } + sort.Strings(types) return strings.Join(types, ", ") } diff --git a/module/openapi_test.go b/module/openapi_test.go index c7c867e0..00942366 100644 --- a/module/openapi_test.go +++ b/module/openapi_test.go @@ -163,6 +163,35 @@ func TestOpenAPIModule_ParseJSON(t *testing.T) { } } +func TestOpenAPIModule_JSONSourceNoYAMLEndpoint(t *testing.T) { + specPath := writeTempSpec(t, ".json", petstoreJSON) + + mod := NewOpenAPIModule("json-api", OpenAPIConfig{ + SpecFile: specPath, + BasePath: "/api", + }) + if err := mod.Init(nil); err != nil { + t.Fatalf("Init: %v", err) + } + + router := &testRouter{} + mod.RegisterRoutes(router) + + paths := make(map[string]bool) + for _, rt := range router.routes { + paths[rt.method+":"+rt.path] = true + } + + // JSON source spec: /openapi.json should be registered + if !paths["GET:/api/openapi.json"] { + t.Error("expected GET:/api/openapi.json to be registered for JSON source") + } + // /openapi.yaml should NOT be registered for a JSON source spec + if paths["GET:/api/openapi.yaml"] { + t.Error("expected GET:/api/openapi.yaml NOT to be registered for JSON source") + } +} + func TestOpenAPIModule_MissingSpecFile(t *testing.T) { mod := NewOpenAPIModule("bad", OpenAPIConfig{}) if err := mod.Init(nil); err == nil { @@ -429,6 +458,41 @@ func TestOpenAPIModule_RequestValidation_Body(t *testing.T) { }) } +func TestOpenAPIModule_MaxBodySize(t *testing.T) { + specPath := writeTempSpec(t, ".yaml", petstoreYAML) + + mod := NewOpenAPIModule("petstore", OpenAPIConfig{ + SpecFile: specPath, + BasePath: "/api/v1", + Validation: OpenAPIValidationConfig{Request: true}, + MaxBodyBytes: 10, // very small limit to trigger the check + }) + if err := mod.Init(nil); err != nil { + t.Fatalf("Init: %v", err) + } + + router := &testRouter{} + mod.RegisterRoutes(router) + + h := router.findHandler("POST", "/api/v1/pets") + if h == nil { + t.Fatal("POST /api/v1/pets handler not found") + } + + body := `{"name": "Fluffy", "tag": "cat"}` // 33 bytes, exceeds limit of 10 + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/api/v1/pets", bytes.NewBufferString(body)) + r.Header.Set("Content-Type", "application/json") + h.Handle(w, r) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 for oversized body, got %d: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "exceeds maximum") { + t.Errorf("expected error message about size limit, got: %s", w.Body.String()) + } +} + func TestOpenAPIModule_NoValidation(t *testing.T) { specPath := writeTempSpec(t, ".yaml", petstoreYAML) diff --git a/plugins/openapi/plugin.go b/plugins/openapi/plugin.go index dd8d4be5..5da7840e 100644 --- a/plugins/openapi/plugin.go +++ b/plugins/openapi/plugin.go @@ -77,6 +77,13 @@ func (p *Plugin) ModuleFactories() map[string]plugin.ModuleFactory { if v, ok := cfg["router"].(string); ok { oacfg.RouterName = v } + if v, ok := cfg["max_body_bytes"].(int); ok && v > 0 { + oacfg.MaxBodyBytes = int64(v) + } else if v, ok := cfg["max_body_bytes"].(int64); ok && v > 0 { + oacfg.MaxBodyBytes = v + } else if v, ok := cfg["max_body_bytes"].(float64); ok && v > 0 { + oacfg.MaxBodyBytes = int64(v) + } if valCfg, ok := cfg["validation"].(map[string]any); ok { if v, ok2 := valCfg["request"].(bool); ok2 { @@ -151,9 +158,16 @@ func (p *Plugin) ModuleSchemas() []*schema.ModuleSchema { DefaultValue: map[string]any{"enabled": false, "path": "/docs"}, Group: "swagger_ui", }, + { + Key: "max_body_bytes", + Label: "Max Body Size", + Type: schema.FieldTypeNumber, + Description: "Maximum allowed request body size in bytes when validation is enabled (default: 1048576 = 1 MiB)", + Placeholder: "1048576", + Group: "validation", + }, }, DefaultConfig: map[string]any{ - "base_path": "/api/v1", "validation": map[string]any{"request": true, "response": false}, "swagger_ui": map[string]any{"enabled": false, "path": "/docs"}, }, @@ -178,12 +192,16 @@ func (p *Plugin) WiringHooks() []plugin.WiringHook { func wireOpenAPIRoutes(app modular.Application, cfg *config.WorkflowConfig) error { // Build name→router lookup from config dependsOn routerNames := make(map[string]bool) - openAPIDeps := make(map[string][]string) // openapi module name → dependsOn + serverToRouter := make(map[string]string) // http.server name → router name + openAPIDeps := make(map[string][]string) // openapi module name → dependsOn for _, modCfg := range cfg.Modules { - if modCfg.Type == "http.router" { + switch modCfg.Type { + case "http.router": routerNames[modCfg.Name] = true - } - if modCfg.Type == "openapi" { + for _, dep := range modCfg.DependsOn { + serverToRouter[dep] = modCfg.Name + } + case "openapi": openAPIDeps[modCfg.Name] = modCfg.DependsOn } } @@ -214,7 +232,7 @@ func wireOpenAPIRoutes(app modular.Application, cfg *config.WorkflowConfig) erro targetRouter = routers[rName] } - // 2) dependsOn router reference + // 2) dependsOn: direct router reference if targetRouter == nil { for _, dep := range openAPIDeps[oaMod.Name()] { if routerNames[dep] { @@ -226,7 +244,19 @@ func wireOpenAPIRoutes(app modular.Application, cfg *config.WorkflowConfig) erro } } - // 3) Fall back to first available router + // 3) dependsOn: server reference → follow server→router mapping + if targetRouter == nil { + for _, dep := range openAPIDeps[oaMod.Name()] { + if rName, ok := serverToRouter[dep]; ok { + if router, found := routers[rName]; found { + targetRouter = router + break + } + } + } + } + + // 4) Fall back to first available router if targetRouter == nil { targetRouter = firstRouter }