From 7d8c46faa62f77633d865094b4874c1e7a249460 Mon Sep 17 00:00:00 2001 From: Dave Shanley Date: Wed, 31 Dec 2025 14:11:39 -0500 Subject: [PATCH 1/2] Added line / col and security scheme tracking. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After witing this up into wiretap, I realized there are a couple of small gaps. This is a breaking change, but as I just released 10x last night, and it’s new years eve, I am going to say that no-one alive cares. --- config/config.go | 1 + errors/strict_errors.go | 4 + errors/strict_errors_test.go | 12 + helpers/parameter_utilities.go | 53 +++++ helpers/parameter_utilities_test.go | 340 +++++++++++++++++++++++++++ parameters/header_parameters.go | 14 +- parameters/header_parameters_test.go | 258 ++++++++++++++++++++ requests/validate_request.go | 2 + responses/validate_response.go | 2 + strict/polymorphic.go | 9 +- strict/schema_walker.go | 2 +- strict/types.go | 29 ++- strict/types_test.go | 120 ++++++++++ strict/validator.go | 13 +- strict/validator_test.go | 94 +++++++- 15 files changed, 940 insertions(+), 13 deletions(-) create mode 100644 strict/types_test.go diff --git a/config/config.go b/config/config.go index c2d384c..f46acce 100644 --- a/config/config.go +++ b/config/config.go @@ -230,6 +230,7 @@ var defaultIgnoredHeaders = []string{ "if-match", "if-none-match", "if-modified-since", "last-modified", "transfer-encoding", "vary", "x-forwarded-for", "x-forwarded-proto", "x-real-ip", "x-request-id", + "request-start-time", // Added by some API clients for timing } // GetEffectiveStrictIgnoredHeaders returns the list of headers to ignore diff --git a/errors/strict_errors.go b/errors/strict_errors.go index 8455e87..aac6e3c 100644 --- a/errors/strict_errors.go +++ b/errors/strict_errors.go @@ -28,6 +28,8 @@ func UndeclaredPropertyError( direction string, requestPath string, requestMethod string, + specLine int, + specCol int, ) *ValidationError { dirStr := direction if dirStr == "" { @@ -47,6 +49,8 @@ func UndeclaredPropertyError( RequestMethod: requestMethod, ParameterName: name, Context: truncateForContext(value), + SpecLine: specLine, + SpecCol: specCol, } } diff --git a/errors/strict_errors_test.go b/errors/strict_errors_test.go index 6fc3624..160ef1f 100644 --- a/errors/strict_errors_test.go +++ b/errors/strict_errors_test.go @@ -18,6 +18,8 @@ func TestUndeclaredPropertyError(t *testing.T) { "request", "/users", "POST", + 42, + 10, ) assert.NotNil(t, err) @@ -30,6 +32,8 @@ func TestUndeclaredPropertyError(t *testing.T) { assert.Equal(t, "/users", err.RequestPath) assert.Equal(t, "POST", err.RequestMethod) assert.Equal(t, "extra", err.ParameterName) + assert.Equal(t, 42, err.SpecLine) + assert.Equal(t, 10, err.SpecCol) } func TestUndeclaredPropertyError_Response(t *testing.T) { @@ -41,12 +45,16 @@ func TestUndeclaredPropertyError_Response(t *testing.T) { "response", "/items/123", "GET", + 100, + 5, ) assert.NotNil(t, err) assert.Contains(t, err.Message, "response property 'undeclared'") assert.Contains(t, err.Reason, "id, name") assert.Equal(t, "{...}", err.Context) // Map truncated + assert.Equal(t, 100, err.SpecLine) + assert.Equal(t, 5, err.SpecCol) } func TestUndeclaredPropertyError_EmptyDirection(t *testing.T) { @@ -58,9 +66,13 @@ func TestUndeclaredPropertyError_EmptyDirection(t *testing.T) { "", // Empty direction defaults to "request" "/test", "POST", + 0, // Zero values for unknown location + 0, ) assert.Contains(t, err.Message, "request property") + assert.Equal(t, 0, err.SpecLine) + assert.Equal(t, 0, err.SpecCol) } func TestUndeclaredHeaderError(t *testing.T) { diff --git a/helpers/parameter_utilities.go b/helpers/parameter_utilities.go index ca07c2b..69d2915 100644 --- a/helpers/parameter_utilities.go +++ b/helpers/parameter_utilities.go @@ -105,6 +105,59 @@ func ExtractSecurityForOperation(request *http.Request, item *v3.PathItem) []*ba return schemes } +// ExtractSecurityHeaderNames extracts header names from applicable security schemes. +// Returns header names from apiKey schemes with in:"header", plus "Authorization" +// for http/oauth2/openIdConnect schemes. +// +// This function is used by strict mode validation to recognize security headers +// as "declared" headers that should not trigger undeclared header errors. +func ExtractSecurityHeaderNames( + security []*base.SecurityRequirement, + securitySchemes map[string]*v3.SecurityScheme, +) []string { + if security == nil || securitySchemes == nil { + return nil + } + + seen := make(map[string]bool) + var headers []string + + for _, sec := range security { + if sec == nil || sec.ContainsEmptyRequirement { + continue // No security required for this option + } + + if sec.Requirements == nil { + continue + } + + for pair := sec.Requirements.First(); pair != nil; pair = pair.Next() { + schemeName := pair.Key() + scheme, ok := securitySchemes[schemeName] + if !ok || scheme == nil { + continue + } + + var headerName string + switch strings.ToLower(scheme.Type) { + case "apikey": + if strings.ToLower(scheme.In) == Header { + headerName = scheme.Name + } + case "http", "oauth2", "openidconnect": + headerName = "Authorization" + } + + if headerName != "" && !seen[strings.ToLower(headerName)] { + seen[strings.ToLower(headerName)] = true + headers = append(headers, headerName) + } + } + } + + return headers +} + func cast(v string) any { if v == "true" || v == "false" { b, _ := strconv.ParseBool(v) diff --git a/helpers/parameter_utilities_test.go b/helpers/parameter_utilities_test.go index 9a39878..ece384d 100644 --- a/helpers/parameter_utilities_test.go +++ b/helpers/parameter_utilities_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/pb33f/libopenapi/datamodel/high/base" + "github.com/pb33f/libopenapi/orderedmap" "github.com/stretchr/testify/require" v3 "github.com/pb33f/libopenapi/datamodel/high/v3" @@ -115,6 +116,345 @@ func TestExtractSecurityForOperation(t *testing.T) { } } +// Test ExtractSecurityHeaderNames with various security scheme types +func TestExtractSecurityHeaderNames(t *testing.T) { + t.Run("nil inputs", func(t *testing.T) { + require.Nil(t, ExtractSecurityHeaderNames(nil, nil)) + require.Nil(t, ExtractSecurityHeaderNames([]*base.SecurityRequirement{}, nil)) + require.Nil(t, ExtractSecurityHeaderNames(nil, map[string]*v3.SecurityScheme{})) + }) + + t.Run("apiKey with in:header", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "apiKey", + In: "header", + Name: "X-API-Key", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "ApiKeyAuth": {"read"}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"X-API-Key"}, headers) + }) + + t.Run("apiKey with in:query should not add header", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyQuery": { + Type: "apiKey", + In: "query", + Name: "api_key", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "ApiKeyQuery": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("apiKey with in:cookie should not add header", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyCookie": { + Type: "apiKey", + In: "cookie", + Name: "session_id", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "ApiKeyCookie": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("http bearer scheme", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "BearerAuth": { + Type: "http", + Scheme: "bearer", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "BearerAuth": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"Authorization"}, headers) + }) + + t.Run("http basic scheme", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "BasicAuth": { + Type: "http", + Scheme: "basic", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "BasicAuth": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"Authorization"}, headers) + }) + + t.Run("oauth2 scheme", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "OAuth2": { + Type: "oauth2", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "OAuth2": {"read:users"}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"Authorization"}, headers) + }) + + t.Run("openIdConnect scheme", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "OpenID": { + Type: "openIdConnect", + OpenIdConnectUrl: "https://example.com/.well-known/openid-configuration", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "OpenID": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"Authorization"}, headers) + }) + + t.Run("empty security requirement (ContainsEmptyRequirement)", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "apiKey", + In: "header", + Name: "X-API-Key", + }, + } + security := []*base.SecurityRequirement{ + { + ContainsEmptyRequirement: true, + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("nil security requirement in slice", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "apiKey", + In: "header", + Name: "X-API-Key", + }, + } + security := []*base.SecurityRequirement{nil} + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("security requirement with nil Requirements map", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "apiKey", + In: "header", + Name: "X-API-Key", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: nil, + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("multiple security options OR - different headers", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "apiKey", + In: "header", + Name: "X-API-Key", + }, + "BearerAuth": { + Type: "http", + Scheme: "bearer", + }, + } + // OR logic: separate security requirements + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "ApiKeyAuth": {}, + }), + }, + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "BearerAuth": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Len(t, headers, 2) + require.Contains(t, headers, "X-API-Key") + require.Contains(t, headers, "Authorization") + }) + + t.Run("combined requirements AND - both headers", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "apiKey", + In: "header", + Name: "X-API-Key", + }, + "BearerAuth": { + Type: "http", + Scheme: "bearer", + }, + } + // AND logic: multiple schemes in one requirement + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "ApiKeyAuth": {}, + "BearerAuth": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Len(t, headers, 2) + require.Contains(t, headers, "X-API-Key") + require.Contains(t, headers, "Authorization") + }) + + t.Run("security scheme not found in schemes map", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "SomeOtherScheme": { + Type: "apiKey", + In: "header", + Name: "X-Other", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "NonExistent": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("nil scheme in schemes map", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "NilScheme": nil, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "NilScheme": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) + + t.Run("deduplication of Authorization header", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "BearerAuth": { + Type: "http", + Scheme: "bearer", + }, + "OAuth2": { + Type: "oauth2", + }, + } + // Both use Authorization header + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "BearerAuth": {}, + }), + }, + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "OAuth2": {"read"}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"Authorization"}, headers) + }) + + t.Run("case insensitive type matching", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "ApiKeyAuth": { + Type: "APIKEY", // uppercase + In: "HEADER", // uppercase + Name: "X-API-Key", + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "ApiKeyAuth": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Equal(t, []string{"X-API-Key"}, headers) + }) + + t.Run("unknown security type is ignored", func(t *testing.T) { + schemes := map[string]*v3.SecurityScheme{ + "Unknown": { + Type: "mutualTLS", // valid OpenAPI type but doesn't use headers + }, + } + security := []*base.SecurityRequirement{ + { + Requirements: orderedmap.ToOrderedMap(map[string][]string{ + "Unknown": {}, + }), + }, + } + headers := ExtractSecurityHeaderNames(security, schemes) + require.Nil(t, headers) + }) +} + func TestConstructParamMapFromDeepObjectEncoding(t *testing.T) { // Define mock values for testing values := []*QueryParam{ diff --git a/parameters/header_parameters.go b/parameters/header_parameters.go index 6fe8a7f..0dfbcc9 100644 --- a/parameters/header_parameters.go +++ b/parameters/header_parameters.go @@ -203,7 +203,19 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, // strict mode: check for undeclared headers if v.options.StrictMode { - undeclaredHeaders := strict.ValidateRequestHeaders(request.Header, params, v.options) + // Extract security headers applicable to this operation + var securityHeaders []string + if v.document.Components != nil && v.document.Components.SecuritySchemes != nil { + security := helpers.ExtractSecurityForOperation(request, pathItem) + // Convert orderedmap to regular map for the helper + schemesMap := make(map[string]*v3.SecurityScheme) + for pair := v.document.Components.SecuritySchemes.First(); pair != nil; pair = pair.Next() { + schemesMap[pair.Key()] = pair.Value() + } + securityHeaders = helpers.ExtractSecurityHeaderNames(security, schemesMap) + } + + undeclaredHeaders := strict.ValidateRequestHeaders(request.Header, params, securityHeaders, v.options) for _, undeclared := range undeclaredHeaders { validationErrors = append(validationErrors, errors.UndeclaredHeaderError( diff --git a/parameters/header_parameters_test.go b/parameters/header_parameters_test.go index 734d658..8d69c35 100644 --- a/parameters/header_parameters_test.go +++ b/parameters/header_parameters_test.go @@ -1159,3 +1159,261 @@ paths: assert.True(t, valid) assert.Len(t, errors, 0) } + +func TestNewValidator_HeaderParams_StrictMode_SecuritySchemeApiKeyHeader(t *testing.T) { + // Test that apiKey security scheme headers are recognized in strict mode + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /secure/resource: + get: + security: + - ApiKeyAuth: [] + responses: + "200": + description: OK +components: + securitySchemes: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-Key` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/secure/resource", nil) + request.Header.Set("X-API-Key", "my-secret-key") + + valid, errors := v.ValidateHeaderParams(request) + + // X-API-Key should be recognized as a valid header due to security scheme + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_HeaderParams_StrictMode_SecuritySchemeApiKeyHeader_CaseInsensitive(t *testing.T) { + // Test that apiKey security scheme header matching is case-insensitive + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /secure/resource: + get: + security: + - ApiKeyAuth: [] + responses: + "200": + description: OK +components: + securitySchemes: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-KEY` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/secure/resource", nil) + request.Header.Set("x-api-key", "my-secret-key") // lowercase in request + + valid, errors := v.ValidateHeaderParams(request) + + // x-api-key should match X-API-KEY case-insensitively + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_HeaderParams_StrictMode_SecuritySchemeNotApplied(t *testing.T) { + // Test that security scheme headers are NOT recognized if the scheme is not applied to the operation + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /public/resource: + get: + # No security defined for this operation + responses: + "200": + description: OK +components: + securitySchemes: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-Key` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/public/resource", nil) + request.Header.Set("X-API-Key", "my-secret-key") + + valid, errors := v.ValidateHeaderParams(request) + + // X-API-Key should be flagged as undeclared since the security scheme is not applied to this operation + assert.False(t, valid) + assert.Len(t, errors, 1) + assert.Contains(t, errors[0].Message, "X-Api-Key") + assert.Contains(t, errors[0].Message, "not declared") +} + +func TestNewValidator_HeaderParams_StrictMode_MultipleSecurity_OR(t *testing.T) { + // Test multiple security options (OR logic) - any header is valid + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /secure/resource: + get: + security: + - ApiKeyAuth: [] + - BearerAuth: [] + responses: + "200": + description: OK +components: + securitySchemes: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-Key + BearerAuth: + type: http + scheme: bearer` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + // Request with X-API-Key only + request, _ := http.NewRequest(http.MethodGet, "https://things.com/secure/resource", nil) + request.Header.Set("X-API-Key", "my-key") + + valid, errors := v.ValidateHeaderParams(request) + assert.True(t, valid) + assert.Len(t, errors, 0) + + // Request with both (both should be allowed) + request2, _ := http.NewRequest(http.MethodGet, "https://things.com/secure/resource", nil) + request2.Header.Set("X-API-Key", "my-key") + request2.Header.Set("Authorization", "Bearer token") // Authorization is in default ignored headers anyway + + valid2, errors2 := v.ValidateHeaderParams(request2) + assert.True(t, valid2) + assert.Len(t, errors2, 0) +} + +func TestNewValidator_HeaderParams_StrictMode_ApiKeyQuery_NotHeader(t *testing.T) { + // Test that apiKey with in:query does NOT add a header allowance + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /secure/resource: + get: + security: + - ApiKeyQuery: [] + responses: + "200": + description: OK +components: + securitySchemes: + ApiKeyQuery: + type: apiKey + in: query + name: api_key` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/secure/resource", nil) + request.Header.Set("X-Unknown", "value") // No security headers expected + + valid, errors := v.ValidateHeaderParams(request) + + // X-Unknown should be flagged + assert.False(t, valid) + assert.Len(t, errors, 1) + assert.Contains(t, errors[0].Message, "X-Unknown") +} + +func TestNewValidator_HeaderParams_StrictMode_CombinedParamsAndSecurity(t *testing.T) { + // Test that both params and security scheme headers are recognized + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /secure/resource: + get: + security: + - ApiKeyAuth: [] + parameters: + - name: X-Request-Id + in: header + required: true + schema: + type: string + responses: + "200": + description: OK +components: + securitySchemes: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-Key` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/secure/resource", nil) + request.Header.Set("X-Request-Id", "123") + request.Header.Set("X-API-Key", "my-key") + + valid, errors := v.ValidateHeaderParams(request) + + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_HeaderParams_StrictMode_NoComponents(t *testing.T) { + // Test that validation works when there are no components/securitySchemes + spec := `openapi: 3.1.0 +info: + title: Test API + version: "1.0" +paths: + /resource: + get: + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model, config.WithStrictMode()) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/resource", nil) + request.Header.Set("X-Custom", "value") + + valid, errors := v.ValidateHeaderParams(request) + + // X-Custom should be flagged as undeclared + assert.False(t, valid) + assert.Len(t, errors, 1) + assert.Contains(t, errors[0].Message, "X-Custom") +} diff --git a/requests/validate_request.go b/requests/validate_request.go index 0f5b9c4..1e7d828 100644 --- a/requests/validate_request.go +++ b/requests/validate_request.go @@ -342,6 +342,8 @@ func ValidateRequestSchema(input *ValidateRequestSchemaInput) (bool, []*errors.V undeclared.Direction.String(), request.URL.Path, request.Method, + undeclared.SpecLine, + undeclared.SpecCol, )) } } diff --git a/responses/validate_response.go b/responses/validate_response.go index 5ebd255..04bc78d 100644 --- a/responses/validate_response.go +++ b/responses/validate_response.go @@ -357,6 +357,8 @@ func ValidateResponseSchema(input *ValidateResponseSchemaInput) (bool, []*errors undeclared.Direction.String(), request.URL.Path, request.Method, + undeclared.SpecLine, + undeclared.SpecCol, )) } } diff --git a/strict/polymorphic.go b/strict/polymorphic.go index e2617ba..d229f33 100644 --- a/strict/polymorphic.go +++ b/strict/polymorphic.go @@ -123,7 +123,7 @@ func (v *Validator) validateAllOf(ctx *traversalContext, schema *base.Schema, da // Not declared - report as undeclared undeclared = append(undeclared, - newUndeclaredProperty(propPath, propName, propValue, getDeclaredPropertyNames(allDeclared), ctx.direction)) + newUndeclaredProperty(propPath, propName, propValue, getDeclaredPropertyNames(allDeclared), ctx.direction, schema)) } return undeclared @@ -228,8 +228,13 @@ func (v *Validator) validateVariantWithParent(ctx *traversalContext, parent *bas } // Not declared - report as undeclared + // Use variant schema location if available, otherwise fall back to parent + locationSchema := variant + if locationSchema == nil || locationSchema.GoLow() == nil { + locationSchema = parent + } undeclared = append(undeclared, - newUndeclaredProperty(propPath, propName, propValue, getDeclaredPropertyNames(allDeclared), ctx.direction)) + newUndeclaredProperty(propPath, propName, propValue, getDeclaredPropertyNames(allDeclared), ctx.direction, locationSchema)) } return undeclared diff --git a/strict/schema_walker.go b/strict/schema_walker.go index 0489795..042a91c 100644 --- a/strict/schema_walker.go +++ b/strict/schema_walker.go @@ -66,7 +66,7 @@ func (v *Validator) validateObject(ctx *traversalContext, schema *base.Schema, d if !isPropertyDeclared(propName, declared, patterns) { undeclared = append(undeclared, - newUndeclaredProperty(propPath, propName, propValue, getDeclaredPropertyNames(declared), ctx.direction)) + newUndeclaredProperty(propPath, propName, propValue, getDeclaredPropertyNames(declared), ctx.direction, schema)) // even if undeclared, recurse into additionalProperties schema if present if schema.AdditionalProperties != nil && schema.AdditionalProperties.IsA() { diff --git a/strict/types.go b/strict/types.go index 163cf80..f78867e 100644 --- a/strict/types.go +++ b/strict/types.go @@ -88,10 +88,33 @@ type UndeclaredValue struct { // Direction indicates whether this was in a request or response. // used for error message disambiguation when Path is "$.body". Direction Direction + + // SpecLine is the line number in the OpenAPI spec where the parent + // schema is defined. Zero if unavailable. + SpecLine int + + // SpecCol is the column number in the OpenAPI spec where the parent + // schema is defined. Zero if unavailable. + SpecCol int +} + +// extractSchemaLocation extracts the line and column from a schema's low-level +// representation. returns (0, 0) if the schema is nil or has no low-level info. +func extractSchemaLocation(schema *base.Schema) (line, col int) { + if schema == nil { + return 0, 0 + } + low := schema.GoLow() + if low == nil || low.RootNode == nil { + return 0, 0 + } + return low.RootNode.Line, low.RootNode.Column } // newUndeclaredProperty creates an UndeclaredValue for an undeclared object property. -func newUndeclaredProperty(path, name string, value any, declaredNames []string, direction Direction) UndeclaredValue { +// the schema parameter is the parent schema where the property would need to be declared. +func newUndeclaredProperty(path, name string, value any, declaredNames []string, direction Direction, schema *base.Schema) UndeclaredValue { + line, col := extractSchemaLocation(schema) return UndeclaredValue{ Path: path, Name: name, @@ -99,10 +122,14 @@ func newUndeclaredProperty(path, name string, value any, declaredNames []string, Type: "property", DeclaredProperties: declaredNames, Direction: direction, + SpecLine: line, + SpecCol: col, } } // newUndeclaredParam creates an UndeclaredValue for an undeclared parameter (query/header/cookie). +// note: parameters don't have SpecLine/SpecCol because they're defined in OpenAPI parameter objects, +// not schema objects. the parameter itself is the issue, not a schema definition. func newUndeclaredParam(path, name string, value any, paramType string, declaredNames []string, direction Direction) UndeclaredValue { return UndeclaredValue{ Path: path, diff --git a/strict/types_test.go b/strict/types_test.go new file mode 100644 index 0000000..ca135f7 --- /dev/null +++ b/strict/types_test.go @@ -0,0 +1,120 @@ +// Copyright 2023-2025 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package strict + +import ( + "testing" + + "github.com/pb33f/libopenapi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExtractSchemaLocation_NilSchema(t *testing.T) { + line, col := extractSchemaLocation(nil) + assert.Equal(t, 0, line) + assert.Equal(t, 0, col) +} + +func TestExtractSchemaLocation_WithValidSchema(t *testing.T) { + spec := `openapi: "3.1.0" +info: + title: Test + version: "1.0" +paths: + /test: + get: + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + properties: + name: + type: string` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + model, errs := doc.BuildV3Model() + require.Empty(t, errs) + + schema := model.Model.Paths.PathItems.GetOrZero("/test").Get.Responses.Codes.GetOrZero("200").Content.GetOrZero("application/json").Schema.Schema() + require.NotNil(t, schema) + + line, col := extractSchemaLocation(schema) + // The schema starts at line 14 (type: object) + assert.Greater(t, line, 0, "line should be greater than 0") + assert.Greater(t, col, 0, "col should be greater than 0") +} + +func TestExtractSchemaLocation_InlineSchema(t *testing.T) { + // Test with a schema that has GoLow() returning non-nil but RootNode nil + // This is hard to construct directly, but we can test the nil case + line, col := extractSchemaLocation(nil) + assert.Equal(t, 0, line) + assert.Equal(t, 0, col) +} + +func TestNewUndeclaredProperty_WithLocation(t *testing.T) { + spec := `openapi: "3.1.0" +info: + title: Test + version: "1.0" +paths: + /test: + post: + requestBody: + content: + application/json: + schema: + type: object + properties: + name: + type: string` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + model, errs := doc.BuildV3Model() + require.Empty(t, errs) + + schema := model.Model.Paths.PathItems.GetOrZero("/test").Post.RequestBody.Content.GetOrZero("application/json").Schema.Schema() + require.NotNil(t, schema) + + undeclared := newUndeclaredProperty( + "$.body.extra", + "extra", + "value", + []string{"name"}, + DirectionRequest, + schema, + ) + + assert.Equal(t, "$.body.extra", undeclared.Path) + assert.Equal(t, "extra", undeclared.Name) + assert.Equal(t, "property", undeclared.Type) + assert.Greater(t, undeclared.SpecLine, 0, "SpecLine should be set") + assert.Greater(t, undeclared.SpecCol, 0, "SpecCol should be set") +} + +func TestNewUndeclaredProperty_WithNilSchema(t *testing.T) { + undeclared := newUndeclaredProperty( + "$.body.extra", + "extra", + "value", + []string{"name"}, + DirectionRequest, + nil, // nil schema + ) + + assert.Equal(t, "$.body.extra", undeclared.Path) + assert.Equal(t, "extra", undeclared.Name) + assert.Equal(t, "property", undeclared.Type) + assert.Equal(t, 0, undeclared.SpecLine, "SpecLine should be 0 for nil schema") + assert.Equal(t, 0, undeclared.SpecCol, "SpecCol should be 0 for nil schema") +} + diff --git a/strict/validator.go b/strict/validator.go index bc7a53d..1917eff 100644 --- a/strict/validator.go +++ b/strict/validator.go @@ -94,9 +94,15 @@ func ValidateQueryParams( // ValidateRequestHeaders checks for undeclared headers in an HTTP request. // Header names are normalized to lowercase for path generation and pattern matching. +// +// The securityHeaders parameter contains header names that are valid due to security +// scheme definitions (e.g., "X-API-Key" for apiKey schemes, "Authorization" for +// http/oauth2/openIdConnect schemes). These headers are considered "declared" even +// though they don't appear in the operation's parameters array. func ValidateRequestHeaders( headers http.Header, declaredParams []*v3.Parameter, + securityHeaders []string, options *config.ValidationOptions, ) []UndeclaredValue { if headers == nil || options == nil || !options.StrictMode { @@ -113,13 +119,18 @@ func ValidateRequestHeaders( } } + // add security scheme headers (case-insensitive) + for _, h := range securityHeaders { + declared[strings.ToLower(h)] = true + } + var undeclared []UndeclaredValue // check each header for headerName := range headers { lowerName := strings.ToLower(headerName) - // skip if declared + // skip if declared (via parameters or security schemes) if declared[lowerName] { continue } diff --git a/strict/validator_test.go b/strict/validator_test.go index 64c44f0..ba56889 100644 --- a/strict/validator_test.go +++ b/strict/validator_test.go @@ -684,7 +684,7 @@ func TestStrictValidator_HeaderIgnorePathsCase(t *testing.T) { "X-Trace": {"abc"}, } - undeclared := ValidateRequestHeaders(headers, nil, opts) + undeclared := ValidateRequestHeaders(headers, nil, nil, opts) assert.Empty(t, undeclared) } @@ -2824,7 +2824,7 @@ paths: } // ValidateRequestHeaders takes http.Header, not *http.Request - undeclared := ValidateRequestHeaders(headers, params, opts) + undeclared := ValidateRequestHeaders(headers, params, nil, opts) assert.Len(t, undeclared, 1) assert.Equal(t, "X-Unknown-Header", undeclared[0].Name) @@ -5080,7 +5080,7 @@ func TestValidateRequestHeaders_DeclaredHeaderSkipped(t *testing.T) { "X-Undeclared": []string{"should-be-flagged"}, } - undeclared := ValidateRequestHeaders(headers, params, opts) + undeclared := ValidateRequestHeaders(headers, params, nil, opts) // Only X-Undeclared should be reported assert.Len(t, undeclared, 1) @@ -5093,16 +5093,16 @@ func TestValidateRequestHeaders_NilOrDisabled(t *testing.T) { headers := http.Header{"X-Custom": []string{"value"}} // Test nil headers - result := ValidateRequestHeaders(nil, params, config.NewValidationOptions(config.WithStrictMode())) + result := ValidateRequestHeaders(nil, params, nil, config.NewValidationOptions(config.WithStrictMode())) assert.Nil(t, result) // Test nil options - result = ValidateRequestHeaders(headers, params, nil) + result = ValidateRequestHeaders(headers, params, nil, nil) assert.Nil(t, result) // Test strict mode disabled opts := config.NewValidationOptions() // strict mode off by default - result = ValidateRequestHeaders(headers, params, opts) + result = ValidateRequestHeaders(headers, params, nil, opts) assert.Nil(t, result) } @@ -5119,13 +5119,93 @@ func TestValidateRequestHeaders_IgnoredHeaderSkipped(t *testing.T) { "X-Custom": []string{"should-be-flagged"}, } - undeclared := ValidateRequestHeaders(headers, params, opts) + undeclared := ValidateRequestHeaders(headers, params, nil, opts) // Only X-Custom should be reported (Content-Type and Authorization are ignored) assert.Len(t, undeclared, 1) assert.Equal(t, "X-Custom", undeclared[0].Name) } +func TestValidateRequestHeaders_SecurityHeadersRecognized(t *testing.T) { + // Covers validator.go:122-125 - security scheme headers are recognized as declared + opts := config.NewValidationOptions(config.WithStrictMode()) + + // No declared params - security headers come from security schemes + params := []*v3.Parameter{} + + // Security headers extracted from security schemes + securityHeaders := []string{"X-API-Key", "X-Custom-Auth"} + + headers := http.Header{ + "X-Api-Key": []string{"my-api-key"}, // matches securityHeaders (case-insensitive) + "X-Custom-Auth": []string{"custom-token"}, // matches securityHeaders + "X-Unknown": []string{"should-be-flagged"}, + } + + undeclared := ValidateRequestHeaders(headers, params, securityHeaders, opts) + + // Only X-Unknown should be reported; X-Api-Key and X-Custom-Auth are recognized as security headers + assert.Len(t, undeclared, 1) + assert.Equal(t, "X-Unknown", undeclared[0].Name) +} + +func TestValidateRequestHeaders_SecurityHeadersCaseInsensitive(t *testing.T) { + // Verify security header matching is case-insensitive + opts := config.NewValidationOptions(config.WithStrictMode()) + + params := []*v3.Parameter{} + securityHeaders := []string{"X-API-KEY"} // uppercase + + headers := http.Header{ + "x-api-key": []string{"my-key"}, // lowercase in request + } + + undeclared := ValidateRequestHeaders(headers, params, securityHeaders, opts) + + // Should not report x-api-key as undeclared (case-insensitive match) + assert.Empty(t, undeclared) +} + +func TestValidateRequestHeaders_BothParamsAndSecurityHeaders(t *testing.T) { + // Test that both params and security headers are recognized + opts := config.NewValidationOptions(config.WithStrictMode()) + + params := []*v3.Parameter{ + {Name: "X-Request-Id", In: "header"}, + } + securityHeaders := []string{"X-API-Key"} + + headers := http.Header{ + "X-Request-Id": []string{"123"}, // declared via params + "X-Api-Key": []string{"key"}, // declared via security schemes + "X-Other": []string{"should-be-flagged"}, + } + + undeclared := ValidateRequestHeaders(headers, params, securityHeaders, opts) + + // Only X-Other should be reported + assert.Len(t, undeclared, 1) + assert.Equal(t, "X-Other", undeclared[0].Name) +} + +func TestValidateRequestHeaders_EmptySecurityHeaders(t *testing.T) { + // Test with empty security headers slice (not nil) + opts := config.NewValidationOptions(config.WithStrictMode()) + + params := []*v3.Parameter{} + securityHeaders := []string{} // empty, not nil + + headers := http.Header{ + "X-Custom": []string{"value"}, + } + + undeclared := ValidateRequestHeaders(headers, params, securityHeaders, opts) + + // X-Custom should be flagged since there are no declared headers + assert.Len(t, undeclared, 1) + assert.Equal(t, "X-Custom", undeclared[0].Name) +} + func TestValidateResponseHeaders_DeclaredHeaderSkipped(t *testing.T) { // Covers validator.go:219-223, 228-230 - declared header handling in response opts := config.NewValidationOptions(config.WithStrictMode()) From 298bd8b48be43f80db8ddb42f1201ffe26af5011 Mon Sep 17 00:00:00 2001 From: Dave Shanley Date: Wed, 31 Dec 2025 14:28:12 -0500 Subject: [PATCH 2/2] fix test coverage and liniting issues --- strict/types_test.go | 17 +++++++++----- strict/validator_test.go | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/strict/types_test.go b/strict/types_test.go index ca135f7..fcd3091 100644 --- a/strict/types_test.go +++ b/strict/types_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel/high/base" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -51,10 +52,17 @@ paths: assert.Greater(t, col, 0, "col should be greater than 0") } -func TestExtractSchemaLocation_InlineSchema(t *testing.T) { - // Test with a schema that has GoLow() returning non-nil but RootNode nil - // This is hard to construct directly, but we can test the nil case - line, col := extractSchemaLocation(nil) +func TestExtractSchemaLocation_SchemaWithNilGoLow(t *testing.T) { + // Create a high-level schema programmatically (no GoLow()) + // This covers types.go:108 where low == nil + schema := &base.Schema{ + Type: []string{"object"}, + } + + // GoLow() returns nil for programmatically created schemas + require.Nil(t, schema.GoLow()) + + line, col := extractSchemaLocation(schema) assert.Equal(t, 0, line) assert.Equal(t, 0, col) } @@ -117,4 +125,3 @@ func TestNewUndeclaredProperty_WithNilSchema(t *testing.T) { assert.Equal(t, 0, undeclared.SpecLine, "SpecLine should be 0 for nil schema") assert.Equal(t, 0, undeclared.SpecCol, "SpecCol should be 0 for nil schema") } - diff --git a/strict/validator_test.go b/strict/validator_test.go index ba56889..c4abfab 100644 --- a/strict/validator_test.go +++ b/strict/validator_test.go @@ -6135,3 +6135,51 @@ components: assert.True(t, result.Valid) assert.Empty(t, result.UndeclaredValues) } + +func TestStrictValidator_ValidateVariantWithParent_VariantNilGoLow(t *testing.T) { + // Covers polymorphic.go:233-234 - fallback to parent when variant.GoLow() is nil + // This tests the defensive code path for programmatically created schemas + + // Build a real parent schema from YAML (has GoLow()) + yml := `openapi: "3.1.0" +info: + title: Test + version: "1.0" +paths: {} +components: + schemas: + ParentSchema: + type: object + properties: + parentProp: + type: string +` + model := buildSchemaFromYAML(t, yml) + parent := getSchema(t, model, "ParentSchema") + require.NotNil(t, parent.GoLow()) + + // Create a variant schema programmatically (no GoLow()) + variant := &base.Schema{ + Type: []string{"object"}, + } + require.Nil(t, variant.GoLow()) + + opts := config.NewValidationOptions(config.WithStrictMode()) + v := NewValidator(opts, 3.1) + + ctx := newTraversalContext(DirectionRequest, nil, "$.body") + + // Data with undeclared property - triggers line 230-237 + data := map[string]any{ + "parentProp": "value", + "undeclared": "triggers undeclared path", + } + + result := v.validateVariantWithParent(ctx, parent, variant, data) + + // Should detect undeclared property and use parent's location (since variant has no GoLow) + assert.Len(t, result, 1) + assert.Equal(t, "undeclared", result[0].Name) + // Location should come from parent (not crash due to nil variant.GoLow()) + assert.Greater(t, result[0].SpecLine, 0) +}