Merging to release-5.8: [TT-11975] Lack of clear error message during failed import when API definition is not valid (#7077)

[buger]

User description

TT-11975 Lack of clear error message during failed import when API definition is not valid (#7077)

User description

TT-11975

Summary	Lack of clear error message during failed import when API definition is not valid
Type	Bug
Status	In Dev
Points	N/A
Labels	codilime_refined, customer_bug, jira_escalated

---

Description

The system fails to properly validate and provide clear feedback when
there's a mismatch between security schemes and security requirements in
an OpenAPI description. Instead of returning a helpful validation error,
the application crashes with a nil pointer dereference in the security
extraction process, resulting in a generic 500 error.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing
  functionality to change)
• Refactoring or add test (improvements in base code or adds test
  coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning
  why it's required
• I would like a code coverage CI quality gate exception and have
  explained why

---

PR Type

Bug fix, Tests

---

Description

• Added explicit validation for OAS security requirements.

• Improved error messages for missing security schemes.

• Prevented nil pointer dereference during OAS import.

• Introduced comprehensive unit tests for security validation.

---

Changes walkthrough 📝

	Relevant files
Bug fix	oas.go Add OAS security requirements validation and error handling apidef/oas/oas.go Added Validate method to OAS for enhanced validation. Implemented validateSecurity to check security requirements. Improved error messages for missing security schemes. Prevented application crash on invalid security configuration. +35/-0
Tests	oas_test.go Add tests for OAS security validation and error reporting apidef/oas/oas_test.go Added unit tests for OAS security validation logic. Covered scenarios for missing, incomplete, and valid security schemes. Ensured error messages are tested for clarity and accuracy. +164/-0

---

Need help?

Type /help how to

... in the comments thread for any questions about PR-Agent
usage.

Check out the documentation
for more information.

---

PR Type

Bug fix, Tests

---

Description

• Added explicit validation for OAS security requirements.

• Improved error messages for missing security schemes.

• Prevented nil pointer dereference during OAS import.

• Introduced comprehensive unit tests for security validation.

---

Changes walkthrough 📝

	Relevant files
Bug fix	oas.go Add OAS security requirements validation and error handling apidef/oas/oas.go Added a new Validate method to the OAS struct for enhanced validation. Implemented validateSecurity to check for missing security schemes in OAS security requirements. Improved error messages for missing or incomplete security schemes. Prevented application crashes due to nil pointer dereference in security extraction. +35/-0
Tests	oas_test.go Add tests for OAS security validation and error reporting apidef/oas/oas_test.go Added unit tests for OAS security validation logic. Covered scenarios for missing, incomplete, and valid security schemes. Ensured error messages are tested for clarity and accuracy. +164/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

API Changes

--- prev.txt	2025-05-28 09:33:02.779191313 +0000
+++ current.txt	2025-05-28 09:32:58.616196775 +0000
@@ -3596,6 +3596,11 @@
 func (s *OAS) UpdateServers(apiURL, oldAPIURL string)
     UpdateServers sets or updates the first servers URL if it matches oldAPIURL.
 
+func (s *OAS) Validate(ctx context.Context, opts ...openapi3.ValidationOption) error
+    Validate validates OAS document by calling openapi3.T.Validate() function.
+    In addition, it validates Security Requirement section and it's requirements
+    by calling OAS.validateSecurity() function.
+
 type OAuth struct {
 	// Enabled activates the OAuth middleware.
 	//

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 7077 - Fully compliant Compliant requirements: When importing an OpenAPI (OAS) definition with invalid or mismatched security requirements, the system should provide a clear, actionable error message. The application should not crash (e.g., nil pointer dereference) if there is a mismatch between security schemes and security requirements. The error message should indicate what is missing or misconfigured in the OAS security section. Validation logic should explicitly check for the presence of required security schemes referenced in security requirements. Comprehensive unit tests should cover scenarios for missing, incomplete, and valid security schemes, and verify error messages.

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Message Clarity The new error messages for missing security schemes should be reviewed for clarity and usefulness to end users, ensuring they are actionable and not overly technical. return errors.New("No components or security schemes present in OAS") } for _, requirement := range s.Security { for key := range requirement { if _, ok := s.Components.SecuritySchemes[key]; !ok { errorMsg := fmt.Sprintf("Missing required Security Scheme '%s' in Components.SecuritySchemes. "+ "For more information please visit https://swagger.io/specification/#security-requirement-object", key) return errors.New(errorMsg) } } Validation Logic Coverage The new validateSecurity logic should be checked to ensure it covers all edge cases, such as multiple security requirements and partial scheme definitions. func (s *OAS) validateSecurity() error { if len(s.Security) == 0 { return nil } if s.Components == nil || s.Components.SecuritySchemes == nil || len(s.Components.SecuritySchemes) == 0 { return errors.New("No components or security schemes present in OAS") } for _, requirement := range s.Security { for key := range requirement { if _, ok := s.Components.SecuritySchemes[key]; !ok { errorMsg := fmt.Sprintf("Missing required Security Scheme '%s' in Components.SecuritySchemes. "+ "For more information please visit https://swagger.io/specification/#security-requirement-object", key) return errors.New(errorMsg) } } } return nil }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Standardize error message for missing scheme The error message for missing security schemes includes a trailing period and a URL, which may hinder test assertions that expect a specific error prefix. Consider removing the period and URL from the error message to ensure consistency with test expectations. apidef/oas/oas.go [466-470] -errorMsg := fmt.Sprintf("Missing required Security Scheme '%s' in Components.SecuritySchemes. "+ - "For more information please visit https://swagger.io/specification/#security-requirement-object", - key) +errorMsg := fmt.Sprintf("Missing required Security Scheme '%s' in Components.SecuritySchemes", key) return errors.New(errorMsg) Suggestion importance[1-10]: 8 __ Why: This suggestion aligns the error message with test expectations and removes unnecessary details, which directly improves test reliability and clarity of error reporting. It addresses a potential source of brittle or failing tests, making it an important fix.	Medium
Possible issue	Preserve error semantics when joining errors If both validationErr and securityErr are nil, errors.Join will return nil as expected. However, if only one is non-nil, errors.Join will still return a wrapped error, which may not be desirable for error handling and comparison. Consider returning the non-nil error directly if only one exists, to preserve error semantics. apidef/oas/oas.go [449] -return errors.Join(validationErr, securityErr) +if validationErr != nil && securityErr != nil { + return errors.Join(validationErr, securityErr) +} +if validationErr != nil { + return validationErr +} +return securityErr Suggestion importance[1-10]: 7 __ Why: This suggestion improves error handling by returning the original error when only one exists, preserving error semantics and making error comparison and handling more predictable. This is a moderate but meaningful improvement to code quality and maintainability.	Medium

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud