diff --git a/config/config.go b/config/config.go index f5aae1c..a9d2b19 100644 --- a/config/config.go +++ b/config/config.go @@ -96,6 +96,7 @@ type ValidationOptions struct { ValidateResponseStatus bool // ValidateResponseStatus rejects undocumented response status codes. RequestDefaults bool // RequestDefaults stages and atomically applies request defaults. StrictServerMatching bool // StrictServerMatching matches scheme, host, port, base path, and server variables. + ValidateDocumentPathParams bool // ValidateDocumentPathParams rejects documents where path templates and declared path parameters do not correspond. bodyDecoders []content.Registration bodyEncoders []content.EncoderRegistration borrowedState bool @@ -223,6 +224,7 @@ func WithExistingOpts(options *ValidationOptions) Option { o.ValidateResponseStatus = options.ValidateResponseStatus o.RequestDefaults = options.RequestDefaults o.StrictServerMatching = options.StrictServerMatching + o.ValidateDocumentPathParams = options.ValidateDocumentPathParams o.StrictMode = options.StrictMode o.StrictIgnorePaths = options.StrictIgnorePaths o.StrictIgnoredHeaders = options.StrictIgnoredHeaders @@ -433,6 +435,13 @@ func WithStrictServerMatching() Option { return func(o *ValidationOptions) { o.StrictServerMatching = true } } +// WithPathParameterDocumentValidation enables document-level validation that each path template +// variable has a corresponding `in: path` parameter, and that every declared `in: path` +// parameter matches a template variable in the path. +func WithPathParameterDocumentValidation() Option { + return func(o *ValidationOptions) { o.ValidateDocumentPathParams = true } +} + // WithSchemaCache sets a custom cache implementation or disables caching if nil. // Pass nil to disable schema caching and skip cache warming during validator initialization. // The default cache is a thread-safe sync.Map wrapper. diff --git a/config/config_test.go b/config/config_test.go index 72544f0..c043430 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -31,6 +31,7 @@ func TestNewValidationOptions_Defaults(t *testing.T) { assert.True(t, opts.OpenAPIMode) // Default is true assert.False(t, opts.AllowScalarCoercion) // Default is false assert.False(t, opts.AllowXMLBodyValidation) // Default is false + assert.False(t, opts.ValidateDocumentPathParams) // Default is false assert.False(t, opts.AllowURLEncodedBodyValidation) // Default is false assert.False(t, opts.ValidateContentParameters) assert.Nil(t, opts.RegexEngine) @@ -236,12 +237,14 @@ func TestMultipleOptions(t *testing.T) { WithFormatAssertions(), WithContentAssertions(), WithXmlBodyValidation(), + WithPathParameterDocumentValidation(), ) assert.True(t, opts.FormatAssertions) assert.True(t, opts.ContentAssertions) assert.True(t, opts.SecurityValidation) assert.True(t, opts.AllowXMLBodyValidation) + assert.True(t, opts.ValidateDocumentPathParams) assert.True(t, opts.OpenAPIMode) // Default is true assert.False(t, opts.AllowScalarCoercion) // Default is false assert.Nil(t, opts.RegexEngine) diff --git a/schema_validation/validate_document.go b/schema_validation/validate_document.go index 0b50dee..e742762 100644 --- a/schema_validation/validate_document.go +++ b/schema_validation/validate_document.go @@ -423,6 +423,22 @@ func ValidateOpenAPIDocumentWithPrecompiled(doc libopenapi.Document, compiledSch } validationErrors = append(validationErrors, documentError) } + + if options.ValidateDocumentPathParams { + model, modelErr := doc.BuildV3Model() + + if model != nil { + validationErrors = append(validationErrors, validatePathParameters(&model.Model)...) + } else { + validationErrors = append(validationErrors, &liberrors.ValidationError{ + ValidationType: helpers.Schema, + Message: "Document does not pass path parameter validation", + Reason: fmt.Sprintf("The document model could not be build: %v", modelErr), + HowToFix: fmt.Sprintf("ensure the document is valid according to the OpenAPI %s spec", doc.GetVersion()), + }) + } + } + if len(validationErrors) > 0 { return false, validationErrors } diff --git a/schema_validation/validate_path_parameters.go b/schema_validation/validate_path_parameters.go new file mode 100644 index 0000000..84b12c1 --- /dev/null +++ b/schema_validation/validate_path_parameters.go @@ -0,0 +1,164 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package schema_validation + +import ( + "fmt" + "regexp" + + "github.com/pb33f/go-yaml" + v3 "github.com/pb33f/libopenapi/datamodel/high/v3" + + liberrors "github.com/pb33f/libopenapi-validator/errors" + "github.com/pb33f/libopenapi-validator/helpers" +) + +var pathTemplateVarRegex = regexp.MustCompile(`\{([^}]+)\}`) + +func validatePathParameters(model *v3.Document) []*liberrors.ValidationError { + if model == nil || model.Paths == nil || model.Paths.PathItems == nil { + return nil + } + + keyNodes := pathKeyNodes(model) + + var validationErrors []*liberrors.ValidationError + for pair := model.Paths.PathItems.First(); pair != nil; pair = pair.Next() { + validationErrors = append(validationErrors, + validatePathItemParameters(pair.Key(), pair.Value(), keyNodes[pair.Key()])...) + } + return validationErrors +} + +func validatePathItemParameters(pathKey string, pathItem *v3.PathItem, keyNode *yaml.Node) []*liberrors.ValidationError { + if pathItem == nil { + return nil + } + + operations := pathItem.GetOperations() + hasOperations := operations != nil && operations.Len() > 0 + hasPathLevelParams := len(pathItem.Parameters) > 0 + + if !hasOperations && !hasPathLevelParams { + return nil + } + + templateVars := extractTemplateVars(pathKey) + + var errs []*liberrors.ValidationError + + reportedExtra := make(map[string]bool) + checkExtra := func(params []*v3.Parameter) { + for _, p := range params { + if p == nil || p.In != helpers.Path || p.Name == "" { + continue + } + if !templateVars[p.Name] && !reportedExtra[p.Name] { + reportedExtra[p.Name] = true + errs = append(errs, unmatchedParameterError(pathKey, p, keyNode)) + } + } + } + + pathLevelParams := pathParamNames(pathItem.Parameters) + checkExtra(pathItem.Parameters) + + if hasOperations { + for op := operations.First(); op != nil; op = op.Next() { + operation := op.Value() + checkExtra(operation.Parameters) + + effective := pathParamNames(operation.Parameters) + for name := range pathLevelParams { + effective[name] = true + } + for name := range templateVars { + if !effective[name] { + errs = append(errs, missingParameterError(pathKey, op.Key(), name, keyNode)) + } + } + } + } else { + for name := range templateVars { + if !pathLevelParams[name] { + errs = append(errs, missingParameterError(pathKey, "", name, keyNode)) + } + } + } + + return errs +} + +func extractTemplateVars(pathKey string) map[string]bool { + vars := make(map[string]bool) + for _, match := range pathTemplateVarRegex.FindAllStringSubmatch(pathKey, -1) { + if len(match) > 1 && match[1] != "" { + vars[match[1]] = true + } + } + return vars +} + +func pathParamNames(params []*v3.Parameter) map[string]bool { + names := make(map[string]bool) + for _, p := range params { + if p != nil && p.In == helpers.Path && p.Name != "" { + names[p.Name] = true + } + } + return names +} + +func pathKeyNodes(model *v3.Document) map[string]*yaml.Node { + nodes := make(map[string]*yaml.Node) + low := model.GoLow() + if low == nil || low.Paths.Value == nil || low.Paths.Value.PathItems == nil { + return nodes + } + for pair := low.Paths.Value.PathItems.First(); pair != nil; pair = pair.Next() { + key := pair.Key() + nodes[key.Value] = key.KeyNode + } + return nodes +} + +func missingParameterError(pathKey, method, varName string, keyNode *yaml.Node) *liberrors.ValidationError { + reason := fmt.Sprintf("Path %q declares template variable %q but no corresponding 'path' parameter is defined", + pathKey, varName) + if method != "" { + reason = fmt.Sprintf("Path %q declares template variable %q but the %s operation defines no corresponding 'path' parameter", + pathKey, varName, method) + } + return newPathParameterError(pathKey, reason, + fmt.Sprintf("Add a parameter with name: %s, in: path, required: true to the path item or operation", varName), + keyNode) +} + +func unmatchedParameterError(pathKey string, param *v3.Parameter, keyNode *yaml.Node) *liberrors.ValidationError { + node := keyNode + if param != nil && param.GoLow() != nil && param.GoLow().GetKeyNode() != nil { + node = param.GoLow().GetKeyNode() + } + return newPathParameterError(pathKey, + fmt.Sprintf("The 'path' parameter %q defined in %q does not match any template variable in the path", + param.Name, pathKey), + fmt.Sprintf("Remove the %q parameter, or add a matching {%s} template variable to the path", param.Name, param.Name), + node) +} + +func newPathParameterError(pathKey, reason, howToFix string, node *yaml.Node) *liberrors.ValidationError { + err := &liberrors.ValidationError{ + ValidationType: helpers.Schema, + ValidationSubType: helpers.DocumentValidation, + Message: "OpenAPI document validation failed", + Reason: reason, + HowToFix: howToFix, + Context: buildJSONPointer([]string{"paths", pathKey}), + } + if node != nil { + err.SpecLine = node.Line + err.SpecCol = node.Column + } + return err +} diff --git a/schema_validation/validate_path_parameters_test.go b/schema_validation/validate_path_parameters_test.go new file mode 100644 index 0000000..13a1193 --- /dev/null +++ b/schema_validation/validate_path_parameters_test.go @@ -0,0 +1,481 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package schema_validation + +import ( + "testing" + + "github.com/pb33f/libopenapi" + v3 "github.com/pb33f/libopenapi/datamodel/high/v3" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" + + "github.com/pb33f/libopenapi-validator/config" +) + +func validateDocWithPathParams(t *testing.T, spec string) (bool, []string) { + t.Helper() + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + valid, errs := ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + reasons := make([]string, 0, len(errs)) + for _, e := range errs { + reasons = append(reasons, e.Reason) + } + return valid, reasons +} + +func TestValidatePathParameters_Valid(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + parameters: + - name: petId + in: path + required: true + schema: + type: string + responses: + "200": + description: OK` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.True(t, valid) + assert.Empty(t, reasons) +} + +func TestValidatePathParameters_PathLevelParamIsValid(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + parameters: + - name: petId + in: path + required: true + schema: + type: string + get: + responses: + "200": + description: OK + delete: + responses: + "204": + description: No Content` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.True(t, valid) + assert.Empty(t, reasons) +} + +func TestValidatePathParameters_TemplateWithoutParameter(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.False(t, valid) + require.Len(t, reasons, 1) + assert.Contains(t, reasons[0], `template variable "petId"`) + assert.Contains(t, reasons[0], "get operation") +} + +func TestValidatePathParameters_ParameterWithoutTemplate(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets: + get: + parameters: + - name: petId + in: path + required: true + schema: + type: string + responses: + "200": + description: OK` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.False(t, valid) + require.Len(t, reasons, 1) + assert.Contains(t, reasons[0], `'path' parameter "petId"`) + assert.Contains(t, reasons[0], "does not match any template variable") +} + +func TestValidatePathParameters_RequiredHandledByBaseSchema(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + parameters: + - name: petId + in: path + schema: + type: string + responses: + "200": + description: OK` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + valid, errs := ValidateOpenAPIDocument(doc) + assert.False(t, valid) + require.NotEmpty(t, errs) + + validWithOpt, errsWithOpt := ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + assert.False(t, validWithOpt) + assert.Len(t, errsWithOpt, len(errs)) +} + +func TestValidatePathParameters_EmptyPathItemIsExempt(t *testing.T) { + spec := `{"openapi":"3.1.0","info":{"title":"Test","version":"1.0.0"},"paths":{"/test/{param}":{}}}` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.True(t, valid) + assert.Empty(t, reasons) +} + +func TestValidatePathParameters_MultipleTemplateVars(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}/toys/{toyId}: + get: + parameters: + - name: petId + in: path + required: true + schema: + type: string + responses: + "200": + description: OK` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.False(t, valid) + require.Len(t, reasons, 1) + assert.Contains(t, reasons[0], `template variable "toyId"`) +} + +func TestValidatePathParameters_OnePerOperationMissing(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + parameters: + - name: petId + in: path + required: true + schema: + type: string + responses: + "200": + description: OK + delete: + responses: + "204": + description: No Content` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.False(t, valid) + require.Len(t, reasons, 1) + assert.Contains(t, reasons[0], "delete operation") +} + +func TestValidatePathParameters_RefParameterResolved(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + parameters: + - $ref: '#/components/parameters/PetId' + responses: + "200": + description: OK +components: + parameters: + PetId: + name: petId + in: path + required: true + schema: + type: string` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.True(t, valid) + assert.Empty(t, reasons) +} + +func TestValidatePathParameters_DisabledByDefault(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + valid, errs := ValidateOpenAPIDocument(doc) + assert.True(t, valid) + assert.Empty(t, errs) +} + +func TestValidatePathParameters_ReportsLineAndContext(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + valid, errs := ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + assert.False(t, valid) + require.Len(t, errs, 1) + assert.Equal(t, 6, errs[0].SpecLine) + assert.Equal(t, "/paths/~1pets~1{petId}", errs[0].Context) + assert.Contains(t, errs[0].HowToFix, "in: path") +} + +func TestValidatePathParameters_NonPathParameterIgnored(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + parameters: + - name: petId + in: path + required: true + schema: + type: string + - name: limit + in: query + schema: + type: integer + responses: + "200": + description: OK` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.True(t, valid) + assert.Empty(t, reasons) +} + +func TestValidatePathParameters_PathLevelParamsNoOperationsMissing(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}/{toyId}: + parameters: + - name: petId + in: path + required: true + schema: + type: string` + + valid, reasons := validateDocWithPathParams(t, spec) + assert.False(t, valid) + require.Len(t, reasons, 1) + assert.Contains(t, reasons[0], `template variable "toyId"`) + assert.Contains(t, reasons[0], "no corresponding 'path' parameter") +} + +func TestValidatePathParameters_NilModelReturnsNil(t *testing.T) { + assert.Nil(t, validatePathParameters(nil)) + assert.Nil(t, validatePathParameters(&v3.Document{})) +} + +func TestValidatePathItemParameters_NilPathItemReturnsNil(t *testing.T) { + assert.Nil(t, validatePathItemParameters("/pets/{petId}", nil, nil)) +} + +func TestPathKeyNodes_NilLowReturnsEmpty(t *testing.T) { + assert.Empty(t, pathKeyNodes(&v3.Document{})) +} + +func TestValidatePathParameters_RunWithModelError(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/DoesNotExist'` + + doc, err := libopenapi.NewDocument([]byte(spec)) + + assert.Nil(t, err) + + valid, errs := ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) + + valid, errs = ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) + + valid, errs = ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) +} + +func TestValidatePathParameters_NoValidationErrorWithoutConfig(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/DoesNotExist'` + + doc, err := libopenapi.NewDocument([]byte(spec)) + + assert.Nil(t, err) + + valid, errs := ValidateOpenAPIDocument(doc) + + assert.True(t, valid) + assert.Len(t, errs, 0) + + valid, errs = ValidateOpenAPIDocument(doc) + + assert.True(t, valid) + assert.Len(t, errs, 0) + + valid, errs = ValidateOpenAPIDocument(doc) + + assert.True(t, valid) + assert.Len(t, errs, 0) +} + +func TestValidatePathParameters_ModelCouldNotBeBuilt(t *testing.T) { + spec := `swagger: "2.0" +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + valid, errs := ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) + assert.Contains(t, errs[0].Reason, "The document model could not be build") +} + +func TestValidatePathParameters_CircularReferencesMustAlwaysFail(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + Node: + type: object + required: + - child + properties: + child: + $ref: '#/components/schemas/Node' +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/Node'` + + doc, err := libopenapi.NewDocument([]byte(spec)) + + assert.Nil(t, err) + + valid, errs := ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) + + valid, errs = ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) + + valid, errs = ValidateOpenAPIDocument(doc, config.WithPathParameterDocumentValidation()) + + assert.False(t, valid) + assert.Len(t, errs, 1) +} diff --git a/validator.go b/validator.go index c5e74ed..d9ff91a 100644 --- a/validator.go +++ b/validator.go @@ -245,6 +245,9 @@ func (v *validator) ValidateDocument() (bool, []*errors.ValidationError) { var validationOpts []config.Option if v.options != nil { validationOpts = append(validationOpts, config.WithRegexEngine(v.options.RegexEngine)) + if v.options.ValidateDocumentPathParams { + validationOpts = append(validationOpts, config.WithPathParameterDocumentValidation()) + } } return schema_validation.ValidateOpenAPIDocument(v.document, validationOpts...) } diff --git a/validator_test.go b/validator_test.go index 6b1602a..04c9f41 100644 --- a/validator_test.go +++ b/validator_test.go @@ -180,6 +180,34 @@ func TestNewValidator_WithRegex(t *testing.T) { assert.Empty(t, valErrs) } +func TestNewValidator_WithPathParameterValidation(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /pets/{petId}: + get: + responses: + "200": + description: OK` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.Nil(t, err, "Failed to load spec") + + v, errs := NewValidator(doc) + require.Empty(t, errs) + valid, _ := v.ValidateDocument() + assert.True(t, valid) + + v, errs = NewValidator(doc, config.WithPathParameterDocumentValidation()) + require.Empty(t, errs) + valid, valErrs := v.ValidateDocument() + assert.False(t, valid) + require.NotEmpty(t, valErrs) + assert.Contains(t, valErrs[0].Reason, `template variable "petId"`) +} + func TestNewValidator_WithCustomFormat_NoErrors(t *testing.T) { spec := `openapi: 3.1.0 paths: