diff --git a/module/openapi.go b/module/openapi.go index 2d541cc9..50130407 100644 --- a/module/openapi.go +++ b/module/openapi.go @@ -102,22 +102,81 @@ type openAPIResponse struct { Content map[string]openAPIMediaType `yaml:"content" json:"content"` } +// openAPIAdditionalProperties represents the OpenAPI / JSON Schema +// "additionalProperties" keyword, which may be either a boolean shorthand +// (true = allow any extra keys, false = forbid extra keys) or a full Schema +// object that every additional key's value must satisfy. +type openAPIAdditionalProperties struct { + // Bool is set when the YAML/JSON value is a plain boolean. + // When Schema is non-nil, Bool is ignored. + Bool bool + Schema *openAPISchema +} + +// UnmarshalYAML handles both the boolean shorthand and the object form. +func (a *openAPIAdditionalProperties) UnmarshalYAML(value *yaml.Node) error { + // Boolean shorthand: additionalProperties: true / false + if value.Kind == yaml.ScalarNode && value.Tag == "!!bool" { + var b bool + if err := value.Decode(&b); err != nil { + return err + } + a.Bool = b + a.Schema = nil + return nil + } + // Object form: additionalProperties: { type: string, … } + var s openAPISchema + if err := value.Decode(&s); err != nil { + return err + } + a.Schema = &s + return nil +} + +// UnmarshalJSON handles both the boolean shorthand and the object form. +func (a *openAPIAdditionalProperties) UnmarshalJSON(data []byte) error { + if len(data) > 0 && (data[0] == 't' || data[0] == 'f') { + var b bool + if err := json.Unmarshal(data, &b); err != nil { + return err + } + a.Bool = b + a.Schema = nil + return nil + } + var s openAPISchema + if err := json.Unmarshal(data, &s); err != nil { + return err + } + a.Schema = &s + return nil +} + +// MarshalJSON serialises back to the compact boolean or object form. +func (a openAPIAdditionalProperties) MarshalJSON() ([]byte, error) { + if a.Schema == nil { + return json.Marshal(a.Bool) + } + return json.Marshal(a.Schema) +} + // openAPISchema is a minimal JSON Schema subset used for parameter/body validation. type openAPISchema struct { - Type string `yaml:"type" json:"type"` - Required []string `yaml:"required" json:"required"` - Properties map[string]*openAPISchema `yaml:"properties" json:"properties"` - Format string `yaml:"format" json:"format"` - Minimum *float64 `yaml:"minimum" json:"minimum"` - Maximum *float64 `yaml:"maximum" json:"maximum"` - MinLength *int `yaml:"minLength" json:"minLength"` - MaxLength *int `yaml:"maxLength" json:"maxLength"` - Pattern string `yaml:"pattern" json:"pattern"` - Enum []any `yaml:"enum" json:"enum"` - Items *openAPISchema `yaml:"items" json:"items"` - MinItems *int `yaml:"minItems" json:"minItems"` - MaxItems *int `yaml:"maxItems" json:"maxItems"` - AdditionalProperties *openAPISchema `yaml:"additionalProperties" json:"additionalProperties"` + Type string `yaml:"type" json:"type"` + Required []string `yaml:"required" json:"required"` + Properties map[string]*openAPISchema `yaml:"properties" json:"properties"` + Format string `yaml:"format" json:"format"` + Minimum *float64 `yaml:"minimum" json:"minimum"` + Maximum *float64 `yaml:"maximum" json:"maximum"` + MinLength *int `yaml:"minLength" json:"minLength"` + MaxLength *int `yaml:"maxLength" json:"maxLength"` + Pattern string `yaml:"pattern" json:"pattern"` + Enum []any `yaml:"enum" json:"enum"` + Items *openAPISchema `yaml:"items" json:"items"` + MinItems *int `yaml:"minItems" json:"minItems"` + MaxItems *int `yaml:"maxItems" json:"maxItems"` + AdditionalProperties *openAPIAdditionalProperties `yaml:"additionalProperties" json:"additionalProperties"` } // ---- OpenAPIModule ---- @@ -982,14 +1041,26 @@ func validateJSONBody(body any, schema *openAPISchema, bodyLabel string) []strin // Validate additionalProperties: keys not declared in Properties are checked // against the additionalProperties schema when it is specified. if schema.AdditionalProperties != nil { - for key, val := range obj { - if _, defined := schema.Properties[key]; defined { - continue + ap := schema.AdditionalProperties + if ap.Schema == nil && !ap.Bool { + // additionalProperties: false — reject any key not listed in Properties + for key := range obj { + if _, defined := schema.Properties[key]; !defined { + errs = append(errs, fmt.Sprintf("%s: additional property %q is not allowed", bodyLabel, key)) + } } - if fieldErrs := validateJSONValue(val, key, schema.AdditionalProperties); len(fieldErrs) > 0 { - errs = append(errs, fieldErrs...) + } else if ap.Schema != nil { + // additionalProperties: — validate each extra key against the schema + for key, val := range obj { + if _, defined := schema.Properties[key]; defined { + continue + } + if fieldErrs := validateJSONValue(val, key, ap.Schema); len(fieldErrs) > 0 { + errs = append(errs, fieldErrs...) + } } } + // additionalProperties: true — any extra key is allowed; nothing to validate } return errs } diff --git a/module/openapi_test.go b/module/openapi_test.go index 276bad30..2683b88f 100644 --- a/module/openapi_test.go +++ b/module/openapi_test.go @@ -11,6 +11,8 @@ import ( "path/filepath" "strings" "testing" + + "gopkg.in/yaml.v3" ) // ---- spec fixtures ---- @@ -2394,3 +2396,304 @@ func TestOpenAPIModule_ResponseValidation_DefaultAction_IsWarn(t *testing.T) { t.Errorf("expected 200 with default warn action, got %d: %s", w.Code, w.Body.String()) } } + +// ---- additionalProperties tests ---- + +// additionalPropertiesTrueYAML exercises additionalProperties: true (bool shorthand). +const additionalPropertiesTrueYAML = ` +openapi: "3.0.0" +info: + title: AdditionalProperties True + version: "1.0.0" +paths: + /metadata: + post: + operationId: postMetadata + summary: Post metadata with any extra keys + x-pipeline: metadata-pipeline + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + additionalProperties: true + responses: + "200": + description: OK +` + +// additionalPropertiesFalseYAML exercises additionalProperties: false (reject extras). +const additionalPropertiesFalseYAML = ` +openapi: "3.0.0" +info: + title: AdditionalProperties False + version: "1.0.0" +paths: + /strict: + post: + operationId: postStrict + summary: Post object with no extra keys allowed + x-pipeline: strict-pipeline + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + additionalProperties: false + responses: + "200": + description: OK +` + +// additionalPropertiesSchemaYAML exercises additionalProperties: . +const additionalPropertiesSchemaYAML = ` +openapi: "3.0.0" +info: + title: AdditionalProperties Schema + version: "1.0.0" +paths: + /typed-extra: + post: + operationId: postTypedExtra + summary: Post object where extra keys must be strings + x-pipeline: typed-extra-pipeline + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + additionalProperties: + type: string + responses: + "200": + description: OK +` + +// TestOpenAPIModule_AdditionalProperties_True verifies that a spec with +// "additionalProperties: true" is parsed without error and that arbitrary +// extra keys in the request body are accepted. +func TestOpenAPIModule_AdditionalProperties_True(t *testing.T) { + specPath := writeTempSpec(t, ".yaml", additionalPropertiesTrueYAML) + + mod := NewOpenAPIModule("ap-true", OpenAPIConfig{ + SpecFile: specPath, + Validation: OpenAPIValidationConfig{Request: true}, + }) + if err := mod.Init(nil); err != nil { + t.Fatalf("Init with additionalProperties:true should succeed, got: %v", err) + } + + step := &stubPipelineStep{ + name: "metadata", + exec: func(_ context.Context, _ *PipelineContext) (*StepResult, error) { + return &StepResult{Output: map[string]any{"response_status": 200}, Stop: true}, nil + }, + } + pipe := &Pipeline{Name: "metadata-pipeline", Steps: []PipelineStep{step}} + mod.SetPipelineLookup(func(name string) (*Pipeline, bool) { + if name == "metadata-pipeline" { + return pipe, true + } + return nil, false + }) + + router := &testRouter{} + mod.RegisterRoutes(router) + + h := router.findHandler("POST", "/metadata") + if h == nil { + t.Fatal("POST /metadata handler not found") + } + + // Body contains declared key "name" plus extra keys — all should pass + body := `{"name":"test","extra_field":"value","another_key":42}` + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/metadata", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + h.Handle(w, r) + + if w.Code != http.StatusOK { + t.Errorf("expected extra keys to pass with additionalProperties:true, got status %d: %s", w.Code, w.Body.String()) + } +} + +// TestOpenAPIModule_AdditionalProperties_False verifies that extra keys are +// rejected when "additionalProperties: false" is set. +func TestOpenAPIModule_AdditionalProperties_False(t *testing.T) { + specPath := writeTempSpec(t, ".yaml", additionalPropertiesFalseYAML) + + mod := NewOpenAPIModule("ap-false", OpenAPIConfig{ + SpecFile: specPath, + Validation: OpenAPIValidationConfig{Request: true}, + }) + if err := mod.Init(nil); err != nil { + t.Fatalf("Init with additionalProperties:false should succeed, got: %v", err) + } + + step := &stubPipelineStep{ + name: "strict", + exec: func(_ context.Context, _ *PipelineContext) (*StepResult, error) { + return &StepResult{Output: map[string]any{"response_status": 200}, Stop: true}, nil + }, + } + pipe := &Pipeline{Name: "strict-pipeline", Steps: []PipelineStep{step}} + mod.SetPipelineLookup(func(name string) (*Pipeline, bool) { + if name == "strict-pipeline" { + return pipe, true + } + return nil, false + }) + + router := &testRouter{} + mod.RegisterRoutes(router) + + h := router.findHandler("POST", "/strict") + if h == nil { + t.Fatal("POST /strict handler not found") + } + + // Body contains extra key "unknown" which should be rejected + body := `{"name":"test","unknown":"value"}` + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/strict", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + h.Handle(w, r) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 for extra key with additionalProperties:false, got %d: %s", w.Code, w.Body.String()) + } +} + +// TestOpenAPIModule_AdditionalProperties_Schema verifies that extra keys are +// validated against the schema when "additionalProperties: {type: string}" is set. +func TestOpenAPIModule_AdditionalProperties_Schema(t *testing.T) { + specPath := writeTempSpec(t, ".yaml", additionalPropertiesSchemaYAML) + + mod := NewOpenAPIModule("ap-schema", OpenAPIConfig{ + SpecFile: specPath, + Validation: OpenAPIValidationConfig{Request: true}, + }) + if err := mod.Init(nil); err != nil { + t.Fatalf("Init with additionalProperties schema should succeed, got: %v", err) + } + + step := &stubPipelineStep{ + name: "typed-extra", + exec: func(_ context.Context, _ *PipelineContext) (*StepResult, error) { + return &StepResult{Output: map[string]any{"response_status": 200}, Stop: true}, nil + }, + } + pipe := &Pipeline{Name: "typed-extra-pipeline", Steps: []PipelineStep{step}} + mod.SetPipelineLookup(func(name string) (*Pipeline, bool) { + if name == "typed-extra-pipeline" { + return pipe, true + } + return nil, false + }) + + router := &testRouter{} + mod.RegisterRoutes(router) + + h := router.findHandler("POST", "/typed-extra") + if h == nil { + t.Fatal("POST /typed-extra handler not found") + } + + // Valid: extra key is a string + body := `{"name":"test","extra":"string-value"}` + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/typed-extra", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + h.Handle(w, r) + + if w.Code != http.StatusOK { + t.Errorf("expected string extra key to pass additionalProperties:{type:string}, got status %d: %s", w.Code, w.Body.String()) + } + + // Invalid: extra key value is an integer, not a string — should be rejected + bodyInvalid := `{"name":"test","extra":42}` + w2 := httptest.NewRecorder() + r2 := httptest.NewRequest(http.MethodPost, "/typed-extra", strings.NewReader(bodyInvalid)) + r2.Header.Set("Content-Type", "application/json") + h.Handle(w2, r2) + + if w2.Code != http.StatusBadRequest { + t.Errorf("expected integer extra key to fail additionalProperties:{type:string}, got status %d: %s", w2.Code, w2.Body.String()) + } +} + +// TestOpenAPIAdditionalProperties_UnmarshalYAML checks that the custom YAML +// unmarshaller handles bool and object forms correctly. +func TestOpenAPIAdditionalProperties_UnmarshalYAML(t *testing.T) { + tests := []struct { + input string + wantBool bool + wantSchema bool // whether Schema should be non-nil + wantType string // expected Schema.Type when wantSchema is true + }{ + {"true", true, false, ""}, + {"false", false, false, ""}, + {"type: string", false, true, "string"}, + {"type: integer", false, true, "integer"}, + } + + for _, tc := range tests { + var ap openAPIAdditionalProperties + if err := yaml.Unmarshal([]byte(tc.input), &ap); err != nil { + t.Errorf("UnmarshalYAML(%q): unexpected error: %v", tc.input, err) + continue + } + if ap.Bool != tc.wantBool { + t.Errorf("UnmarshalYAML(%q): Bool = %v, want %v", tc.input, ap.Bool, tc.wantBool) + } + if (ap.Schema != nil) != tc.wantSchema { + t.Errorf("UnmarshalYAML(%q): Schema non-nil = %v, want %v", tc.input, ap.Schema != nil, tc.wantSchema) + } + if tc.wantSchema && ap.Schema != nil && ap.Schema.Type != tc.wantType { + t.Errorf("UnmarshalYAML(%q): Schema.Type = %q, want %q", tc.input, ap.Schema.Type, tc.wantType) + } + } +} + +// TestOpenAPIAdditionalProperties_UnmarshalJSON checks the JSON unmarshaller. +func TestOpenAPIAdditionalProperties_UnmarshalJSON(t *testing.T) { + tests := []struct { + input string + wantBool bool + wantSchema bool + wantType string + }{ + {`true`, true, false, ""}, + {`false`, false, false, ""}, + {`{"type":"string"}`, false, true, "string"}, + } + + for _, tc := range tests { + var ap openAPIAdditionalProperties + if err := json.Unmarshal([]byte(tc.input), &ap); err != nil { + t.Errorf("UnmarshalJSON(%q): unexpected error: %v", tc.input, err) + continue + } + if ap.Bool != tc.wantBool { + t.Errorf("UnmarshalJSON(%q): Bool = %v, want %v", tc.input, ap.Bool, tc.wantBool) + } + if (ap.Schema != nil) != tc.wantSchema { + t.Errorf("UnmarshalJSON(%q): Schema non-nil = %v, want %v", tc.input, ap.Schema != nil, tc.wantSchema) + } + if tc.wantSchema && ap.Schema != nil && ap.Schema.Type != tc.wantType { + t.Errorf("UnmarshalJSON(%q): Schema.Type = %q, want %q", tc.input, ap.Schema.Type, tc.wantType) + } + } +} diff --git a/module/pipeline_step_graphql.go b/module/pipeline_step_graphql.go index 77cccc3f..a42aa2a1 100644 --- a/module/pipeline_step_graphql.go +++ b/module/pipeline_step_graphql.go @@ -316,8 +316,9 @@ func (s *GraphQLStep) Execute(ctx context.Context, pc *PipelineContext) (*StepRe // Execute request (with optional network error retry) output, statusCode, err := s.doRequest(ctx, resolvedURL, reqBody, bearerToken) if err != nil { - // 401 retry with token refresh - if statusCode == http.StatusUnauthorized && s.auth != nil { + switch { + case statusCode == http.StatusUnauthorized && s.auth != nil: + // 401 retry with token refresh s.oauthEntry.invalidate() bearerToken, err = s.fetchTokenDirect(ctx) if err != nil { @@ -327,13 +328,13 @@ func (s *GraphQLStep) Execute(ctx context.Context, pc *PipelineContext) (*StepRe if err != nil { return nil, err } - } else if s.retryOnNetworkError && statusCode == 0 { + case s.retryOnNetworkError && statusCode == 0: // Network-level error (no HTTP response) — retry once output, statusCode, err = s.doRequest(ctx, resolvedURL, reqBody, bearerToken) if err != nil { return nil, err } - } else { + default: return nil, err } }