fix(openapi): address remaining review comments — validation, content-type, schema, defaults (#150)

* Initial plan

* fix(openapi): address all remaining review comments — body bytes, JSON errors, content-type, schema, defaults, logging

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

Author: Copilot

module/openapi.go

@@ -1,6 +1,7 @@
 package module
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -237,10 +238,14 @@ func (m *OpenAPIModule) RegisterRoutes(router HTTPRouter) {
 		// 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 YAML content-type.
-		// This preserves the source format; if the original file was YAML it is
-		// served as YAML, and if it was JSON it is served as-is (JSON is valid YAML).
-		router.AddRoute(http.MethodGet, specPathYAML, &openAPIRawSpecHandler{specBytes: m.specBytes, contentType: "application/yaml"})
+		// 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"
+		}
+		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})
 	}
@@ -250,6 +255,8 @@ func (m *OpenAPIModule) RegisterRoutes(router HTTPRouter) {
 		uiPath := m.cfg.SwaggerUI.Path
 		if uiPath == "" {
 			uiPath = "/docs"
+		} else if !strings.HasPrefix(uiPath, "/") {
+			uiPath = "/" + uiPath
 		}
 		uiRoutePath := basePath + uiPath
 		specURL := basePath + "/openapi.json"
@@ -352,6 +359,10 @@ func (h *openAPIRouteHandler) validate(r *http.Request) []string {
 			// application/octet-stream, but this engine is primarily used for JSON APIs and this
 			// default simplifies client usage.
 			mediaType = &mt
+		} else if ct != "" && len(h.op.RequestBody.Content) > 0 {
+			// Content-Type is present but not listed in the spec — reject with 400.
+			errs = append(errs, fmt.Sprintf("unsupported Content-Type %q; spec defines: %s",
+				ct, supportedContentTypes(h.op.RequestBody.Content)))
 		}
 
 		// Read the body once so we can both check for presence (when required)
@@ -365,17 +376,18 @@ func (h *openAPIRouteHandler) validate(r *http.Request) []string {
 			)
 			errs = append(errs, "failed to read request body")
 		} else {
-			// Always restore body for downstream handlers.
-			r.Body = io.NopCloser(strings.NewReader(string(bodyBytes)))
+			// 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.
+			r.Body = io.NopCloser(bytes.NewReader(bodyBytes))
 
 			if h.op.RequestBody.Required && len(bodyBytes) == 0 {
 				errs = append(errs, "request body is required but missing")
 			} else if mediaType != nil && mediaType.Schema != nil && len(bodyBytes) > 0 {
 				var bodyData any
-				if jsonErr := json.Unmarshal(bodyBytes, &bodyData); jsonErr == nil {
-					if bodyErrs := validateJSONBody(bodyData, mediaType.Schema); len(bodyErrs) > 0 {
-						errs = append(errs, bodyErrs...)
-					}
+				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 {
+					errs = append(errs, bodyErrs...)
 				}
 			}
 		}
@@ -715,3 +727,13 @@ func swaggerUIHTML(title, specURL string) string {
 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.
+func supportedContentTypes(content map[string]openAPIMediaType) string {
+	types := make([]string, 0, len(content))
+	for ct := range content {
+		types = append(types, ct)
+	}
+	return strings.Join(types, ", ")
+}

module/openapi_test.go

@@ -416,6 +416,17 @@ func TestOpenAPIModule_RequestValidation_Body(t *testing.T) {
 			t.Errorf("expected 400 validation error, got %d: %s", w.Code, w.Body.String())
 		}
 	})
+
+	t.Run("invalid JSON", func(t *testing.T) {
+		body := `{"name": "Fluffy",` // malformed JSON
+		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 validation error for malformed JSON, got %d: %s", w.Code, w.Body.String())
+		}
+	})
 }
 
 func TestOpenAPIModule_NoValidation(t *testing.T) {

plugins/openapi/plugin.go

@@ -4,6 +4,8 @@
 package openapi
 
 import (
+	"log/slog"
+
 	"github.com/CrisisTextLine/modular"
 	"github.com/GoCodeAlone/workflow/capability"
 	"github.com/GoCodeAlone/workflow/config"
@@ -56,7 +58,10 @@ func (p *Plugin) Capabilities() []capability.Contract {
 func (p *Plugin) ModuleFactories() map[string]plugin.ModuleFactory {
 	return map[string]plugin.ModuleFactory{
 		"openapi": func(name string, cfg map[string]any) modular.Module {
-			oacfg := module.OpenAPIConfig{}
+			oacfg := module.OpenAPIConfig{
+				// Default: enable request validation unless explicitly overridden.
+				Validation: module.OpenAPIValidationConfig{Request: true},
+			}
 
 			// NOTE: spec_file existence is not validated here at configuration time.
 			// Path resolution is performed by ResolvePathInConfig (relative to the
@@ -227,7 +232,10 @@ func wireOpenAPIRoutes(app modular.Application, cfg *config.WorkflowConfig) erro
 		}
 
 		if targetRouter == nil {
-			// No router found — log and skip (not fatal; engine may be running without HTTP)
+			// No router found — log a warning and skip (not fatal; engine may be running without HTTP).
+			slog.Warn("openapi: no HTTP router found; skipping route registration",
+				"module", oaMod.Name(),
+			)
 			continue
 		}
 

schema/module_schema.go

@@ -790,11 +790,9 @@ func (r *ModuleSchemaRegistry) registerBuiltins() {
 		ConfigFields: []ConfigFieldDef{
 			{Key: "spec_file", Label: "Spec File", Type: FieldTypeFilePath, Required: true, Description: "Path to the OpenAPI v3 spec file (JSON or YAML)", Placeholder: "specs/petstore.yaml"},
 			{Key: "base_path", Label: "Base Path", Type: FieldTypeString, Description: "Base path prefix for all generated routes", Placeholder: "/api/v1"},
-			{Key: "router", Label: "Router Module", Type: FieldTypeString, Required: true, Description: "Name of the http.router module to register routes on", Placeholder: "my-router"},
-			{Key: "validation.request", Label: "Validate Requests", Type: FieldTypeBool, DefaultValue: true, Description: "Enable request validation against the OpenAPI schema"},
-			{Key: "validation.response", Label: "Validate Responses", Type: FieldTypeBool, DefaultValue: false, Description: "Enable response validation against the OpenAPI schema"},
-			{Key: "swagger_ui.enabled", Label: "Enable Swagger UI", Type: FieldTypeBool, DefaultValue: false, Description: "Serve Swagger UI for interactive API documentation"},
-			{Key: "swagger_ui.path", Label: "Swagger UI Path", Type: FieldTypeString, DefaultValue: "/docs", Description: "URL path for the Swagger UI", Placeholder: "/docs"},
+			{Key: "router", Label: "Router Module", Type: FieldTypeString, Description: "Name of the http.router module to register routes on (auto-detected if omitted)", Placeholder: "my-router"},
+			{Key: "validation", Label: "Validation", Type: FieldTypeJSON, DefaultValue: map[string]any{"request": true, "response": false}, Description: "Request/response validation settings, e.g. {\"request\": true, \"response\": false}"},
+			{Key: "swagger_ui", Label: "Swagger UI", Type: FieldTypeJSON, DefaultValue: map[string]any{"enabled": false, "path": "/docs"}, Description: "Swagger UI settings, e.g. {\"enabled\": false, \"path\": \"/docs\"}"},
 		},
 		DefaultConfig: map[string]any{"validation": map[string]any{"request": true, "response": false}, "swagger_ui": map[string]any{"enabled": false, "path": "/docs"}},
 	})