[TT-16013] Gateway panics when validating JWT claims

[NurayAhmadova]

Description

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

• Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
  fork, don't request your master!
• Make sure you are making a pull request against the master branch (left side). Also, you should start
  your branch off our latest master.
• My change requires a change to the documentation.
  • If you've changed APIs, describe what needs to be updated in the documentation.
  • If new config option added, ensure that it can be set via ENV variable
• I have updated the documentation accordingly.
• Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
• When updating library version must provide reason/explanation for this update.
• I have added tests to cover my changes.
• All new and existing tests passed.
• Check your code additions will not fail linting checks:
  • go fmt -s
  • go vet

Ticket Details

TT-16013

Status	In Dev
Summary	Gateway panics when validating JWT claims

Generated at: 2025-11-27 08:22:39

[github-actions]

🎯 Recommended Merge Targets

Based on JIRA ticket TT-16013: Gateway panics when validating JWT claims

Fix Version: Tyk 5.11.0

⚠️ Warning: Expected release branches not found in repository

Required:

• master - No matching release branches found. Fix will be included in future releases.

---

📋 Workflow

1. Merge this PR to master first

[probelabs]

🔍 Code Analysis Results

{
"text": "This PR addresses a critical race condition (fatal error: concurrent map writes) that could cause the Tyk Gateway to panic. The panic occurred during JWT claim validation for OAS APIs under high load due to unsafe concurrent lazy initialization and modification of the SecuritySchemes map.

The solution replaces the raw map[string]interface{} with a new thread-safe *SecuritySchemes struct. This struct encapsulates the map and protects all access with a sync.RWMutex, ensuring atomic operations and preventing data races. All direct map access has been refactored to use new synchronized methods (Set, Get, Delete, Iter, Len), and initialization is now handled safely via sync.Once.

Files Changed Analysis

The changes are primarily concentrated in the apidef/oas package, with corresponding updates in the gateway middleware and extensive test modifications.

• apidef/oas/authentication.go: This is the core of the change, introducing the new thread-safe SecuritySchemes struct, its constructor NewSecuritySchemes(), and synchronized access methods. It also includes custom MarshalJSON/UnmarshalJSON handlers to maintain wire compatibility.
• apidef/oas/oas.go: Refactors getter functions like getTykJWTAuth to use the new safe methods and introduces a sync.Once for safe, atomic initialization of the security schemes container.
• gateway/mw_auth_or_wrapper.go: The authentication middleware is updated to use the new thread-safe API for retrieving security schemes.
• *_test.go files: Numerous tests across apidef/oas and gateway are updated to use the new *SecuritySchemes struct and its helper constructor, demonstrating thorough testing of the refactored logic.

Architecture & Impact Assessment

• What this PR accomplishes: It resolves a significant stability issue by eliminating a race condition, thereby preventing gateway panics in high-concurrency scenarios involving OAS APIs.

• Key technical changes introduced: The core change is the move from a map[string]interface{} to a *SecuritySchemes struct that enforces thread-safe access to security scheme definitions via a sync.RWMutex. This encapsulates state and concurrency control, making the system more robust.

• Affected system components: The primary components affected are the OAS API Definition loading/parsing logic (apidef/oas) and the Gateway's authentication middleware (gateway), which relies on this logic to pro```mermaid
  graph TD
  subgraph "Before: Unsafe Concurrent Map Access"
  direction LR
  Req1["Request 1"] -->|getTykJWTAuth| Logic1["Lazy Write"]
  Req2["Request 2"] -->|getTykJWTAuth| Logic2["Lazy Write"]
  Logic1 -->|Unsafe Write| SharedMap["map[string]interface{}"]
  Logic2 -->|Unsafe Write| SharedMap
  SharedMap --> Panic["PANIC!"]
  style SharedMap fill:#ffcccc
  end

  subgraph "After: Synchronized Access via Struct"
  direction LR
  ReqA["Request A"] -->|getTykJWTAuth| LogicA["Safe Write via Set()"]
  ReqB["Request B"] -->|getTykJWTAuth| LogicB["Safe Write via Set()"]
  LogicA --> SafeStruct["*SecuritySchemes"]
  LogicB --> SafeStruct
  SafeStruct --|sync.RWMutex|--> InternalMap["(internal map)"]
  style SafeStruct fill:#ccffcc
  end

        style SafeStruct fill:#ccffcc
    end

Scope Discovery & Context Expansion

This PR introduces a breaking change to the internal Go API, although it maintains wire compatibility for JSON/YAML. The Authentication.SecuritySchemes field type has been changed from a map alias to *SecuritySchemes, a pointer to the new struct.

Proof of Breaking Change:

The change is evident in the definition of the Authentication struct in apidef/oas/authentication.go.

Before:

// apidef/oas/authentication.go

// SecuritySchemes was a type alias for a map
type SecuritySchemes map[string]interface{}

type Authentication struct {
    // ...
    SecuritySchemes SecuritySchemes `bson:\"securitySchemes,omitempty\" json:\"securitySchemes,omitempty\"`
    // ...
}

After:

// apidef/oas/authentication.go

// SecuritySchemes is now a struct
type SecuritySchemes struct {
	container map[string]interface{}
	mutex     sync.RWMutex
}

type Authentication struct {
    // ...
    // The field is now a pointer to the struct
	SecuritySchemes *SecuritySchemes `bson:\"securitySchemes,omitempty\" json:\"securitySchemes,omitempty\"`
    // ...
}

This change means that any Go code that consumes this package and directly initializes or assigns to the Authentication.SecuritySchemes field will fail to compile. Code that previously did auth.SecuritySchemes = make(map[string]interface{}) must now be updated to use the constructor auth.SecuritySchemes = NewSecuritySchemes(). This may impact other Tyk projects or internal tools that import this package.",
"tags": {
"review-effort": 3,
"label": "bug"
}
}

---

Powered by Visor from Probelabs

Last updated: 2025-11-19T07:58:19.797Z | Triggered by: synchronize | Commit: f6eb0d8

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

🔍 Code Analysis Results

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Performance Issues (1)

Severity	Location	Issue
🟢 Info	apidef/oas/authentication.go:332	The `Iter()` method creates a snapshot of the security schemes map on every call. This involves allocating a new slice and copying all map entries, which could be a performance concern if an API has a very large number of security schemes. 💡 Suggestion The current implementation is a reasonable trade-off that prioritizes minimizing lock contention over reducing allocations, as noted in the code comment. For the typical use case where the number of security schemes is small, this is an effective strategy. No change is required, but this design choice should be kept in mind if APIs with an unusually high number of security schemes become common.

✅ Quality Check Passed

No quality issues found – changes LGTM.

Dependency Issues (1)

Severity	Location	Issue
🔴 Critical	apidef/oas/authentication.go:80	The `Authentication.SecuritySchemes` field type has been changed from a map alias (`SecuritySchemes`) to a pointer to a struct (`*SecuritySchemes`). This is a breaking change for the internal Go API that will affect downstream consumers like `tyk-operator`, `tyk-portal`, and `tyk-sink` which import this package to work with API definition structs. These dependent projects will fail to compile without updates to accommodate the new type and its constructor (`NewSecuritySchemes()`). 💡 Suggestion To prevent breaking the ecosystem, corresponding pull requests must be created and linked for all affected downstream projects (e.g., `tyk-operator`, `tyk-portal`, `tyk-sink`) that consume these Go structs. This ensures that the breaking change is handled across all dependencies in a coordinated manner.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.

---

Powered by Visor from Probelabs

Last updated: 2025-11-19T07:58:21.149Z | Triggered by: synchronize | Commit: f6eb0d8

💡 TIP: You can chat with Visor using /visor ask <your question>

[github-actions]

API Changes

--- prev.txt	2025-11-27 08:23:05.749089251 +0000
+++ current.txt	2025-11-27 08:22:55.702058828 +0000
@@ -2341,6 +2341,8 @@
 func (ap *AuthenticationPlugin) Fill(api apidef.APIDefinition)
 
 type Basic struct {
+	SecuritySchemeMarkerImpl
+
 	// Enabled activates the basic authentication mode.
 	// Tyk classic API definition: `use_basic_auth`
 	Enabled bool `bson:"enabled" json:"enabled"` // required
@@ -2756,6 +2758,7 @@
     CustomPlugin configures custom plugin.
 
 type CustomPluginAuthentication struct {
+	SecuritySchemeMarkerImpl
 	// Enabled activates the CustomPluginAuthentication authentication mode.
 	//
 	// Tyk classic API definition: `enable_coprocess_auth`/`use_go_plugin_auth`.
@@ -2975,6 +2978,8 @@
     and jsvm events are supported.
 
 type ExternalOAuth struct {
+	SecuritySchemeMarkerImpl
+
 	// Enabled activates external oauth functionality.
 	//
 	// Tyk classic API definition: `external_oauth.enabled`.
@@ -3158,6 +3163,8 @@
     Fill fills *GlobalRequestSizeLimit from apidef.APIDefinition.
 
 type HMAC struct {
+	SecuritySchemeMarkerImpl
+
 	// Enabled activates the HMAC authentication mode.
 	//
 	// Tyk classic API definition: `enable_signature_checking`.
@@ -3446,6 +3453,8 @@
     endpoint and its cache timeout.
 
 type JWT struct {
+	SecuritySchemeMarkerImpl
+
 	// Enabled activates the basic authentication mode.
 	//
 	// Tyk classic API definition: `enable_jwt`
@@ -3856,6 +3865,8 @@
     by calling OAS.validateSecurity() function.
 
 type OAuth struct {
+	SecuritySchemeMarkerImpl
+
 	// Enabled activates the OAuth middleware.
 	//
 	// Tyk classic API definition: `use_oauth2`.
@@ -4437,18 +4448,54 @@
 }
     SecurityScheme defines an Importer interface for security schemes.
 
-type SecuritySchemes map[string]interface{}
-    SecuritySchemes holds security scheme values, filled with Import().
+type SecuritySchemeMarker interface {
+	// Has unexported methods.
+}
+
+type SecuritySchemeMarkerImpl struct{}
+
+type SecuritySchemes struct {
+	// Has unexported fields.
+}
+    SecuritySchemes zero value is usable. Methods lazily initialize internal
+    state. Recommendation: prefer NewSecuritySchemes for clarity and explicit
+    initialization.
+
+func (ss SecuritySchemes) Get(key string) (SecuritySchemeMarker, bool)
+    Get retrieves a security scheme by name. It returns the stored value and
+    true when present, otherwise (nil, false).
 
 func (ss SecuritySchemes) GetBaseIdentityProvider() (res apidef.AuthTypeEnum)
     GetBaseIdentityProvider returns the identity provider by precedence from
     SecuritySchemes.
 
-func (ss SecuritySchemes) Import(name string, nativeSS *openapi3.SecurityScheme, enable bool) error
-    Import takes the openapi3.SecurityScheme as argument and applies it to
-    the receiver. The SecuritySchemes receiver is a map, so modification of
-    the receiver is enabled, regardless of the fact that the receiver isn't a
-    pointer type. The map is a pointer type itself.
+func (ss SecuritySchemes) GetVal(key string) interface{}
+    GetVal retrieves a security scheme by name. It returns the stored value and
+    true when present, otherwise nil.
+
+func (ss SecuritySchemes) Iter() iter.Seq2[string, interface{}]
+    Iter returns a snapshot under a short read lock to avoid holding locks
+    during iteration. This allocates per call. Given the typical small number of
+    schemes, the trade-off is acceptable.
+
+func (ss SecuritySchemes) Len() int
+    Len reports the number of registered security schemes. A nil receiver
+    reports zero.
+
+func (ss *SecuritySchemes) MarshalJSON() ([]byte, error)
+    MarshalJSON implements json.Marshaler.
+
+func (ss *SecuritySchemes) MarshalYAML() (interface{}, error)
+    MarshalYAML makes SecuritySchemes YAML-friendly by exposing its container
+    map.
+
+func (ss SecuritySchemes) Set(key string, value SecuritySchemeMarker) SecuritySchemes
+
+func (ss *SecuritySchemes) UnmarshalJSON(b []byte) error
+    UnmarshalJSON implements json.Unmarshaler.
+
+func (ss *SecuritySchemes) UnmarshalYAML(n *yaml.Node) error
+    UnmarshalYAML populates SecuritySchemes.container from YAML.
 
 type Server struct {
 	// ListenPath is the base path on Tyk to which requests for this API should
@@ -4776,6 +4823,8 @@
     Fill fills *TLSTransport from apidef.ServiceDiscoveryConfiguration.
 
 type Token struct {
+	SecuritySchemeMarkerImpl
+
 	// Enabled activates the token based authentication mode.
 	//
 	// Tyk classic API definition: `auth_configs["authToken"].use_standard_auth`

[lghiur]

/review making SecurityScheme to a pointer is this a breaking change and why? bring proof of why you think it's abreaking change

[shults]

Would you mind fixing race condition in MarshalYAML method?

[shults]

This code does not like to be safe.
Map resize can occur during yaml marshaling.
It would be better to return copy of the map.

[shults]

return reflect.Clone(ss.sontainer), nil

or

return yaml.Marshal(ss.container), nil -> this seams better because of additon allocation is no needed

[shults]

I would rather use testify assert/require, but it's up to you.

[shults]

lgtm

[probelabs]

{
"text": "This pull request aims to resolve a critical panic in the Gateway (fatal error: concurrent map writes) that occurs during JWT claim validation for OAS APIs under high load. The root cause is a race condition from concurrent, unsynchronized access to the SecuritySchemes map.

The proposed solution refactors SecuritySchemes from a map[string]interface{} type alias into a dedicated struct. This change introduces an immutable update pattern, where methods like Set and delete operate on a value receiver and return a new, modified instance of the struct, preventing unsafe in-place mutations.

However, the current implementation is incomplete and does not yet fix the underlying race condition. The immutable update pattern is not consistently applied, with call sites failing to reassign the value returned by modifier methods. This is confirmed by several new tests in apidef/oas/authentication_test.go that are explicitly marked with // TODO: Fails because..., indicating known flaws in the implementation.
\

Files Changed Analysis\

\

• apidef/oas/authentication.go: This is the core of the refactoring. It introduces the new SecuritySchemes struct and its methods (Set, Get, Iter, etc.), which are designed to follow an immutable pattern.\
• apidef/oas/security.go & apidef/oas/oas.go: These files are updated to interact with the new SecuritySchemes struct instead of accessing the map directly.\
• gateway/mw_auth_or_wrapper.go: The authentication middleware is updated to use the new Get method for safely reading security schemes.\
• *_test.go files: Tests across the apidef/oas and gateway packages have been updated. Crucially, several new tests are marked as failing, confirming the implementation is a work-in-progress and does not yet function as intended.
  \

Architecture & Impact Assessment\

\

• What this PR accomplishes: It introduces the structural changes necessary to fix a race condition but does not complete the implementation. In its current state, it does not resolve the gateway panic.\

• Key technical changes introduced: The primary change is the shift from a mutable map to a struct with an immutable update pattern for managing security schemes. This is a significant architectural change aimed at improving concurrency safety.\

• Affected system components: The changes primarily impact the OAS API definition loading/parsing logic (apidef/oas) and the **Gateway's authentication```mermaid
  graph TD
  subgraph "Before: Unsafe Concurrent Map Access"
  direction LR
  Req1["Request 1"] -->|Write| SharedMap["SecuritySchemes (map)"]
  Req2["Request 2"] -->|Write| SharedMap
  SharedMap -->|Race Condition| Panic["PANIC!"]
  style SharedMap fill:#ffcccc
  end

  subgraph "After (Intended Design): Immutable Struct"
  direction LR
  ReqA["Request A"] --|"ss.Set(k, v)"|--> OldStruct["SecuritySchemes (struct)"]
  OldStruct -->|"Returns new instance"| NewStruct["New SecuritySchemes (struct)"]
  style OldStruct fill:#ccffcc
  style NewStruct fill:#ccffcc
  end

        style NewStruct fill:#ccffcc\
    end\
```\
\
### Scope Discovery & Context Expansion\
\
- **Breaking Change**: This PR introduces a significant breaking change to the internal Go API. The type of the `Authentication.SecuritySchemes` field is changed from a map alias to a struct. Any downstream Go projects within the Tyk ecosystem (e.g., `tyk-operator`, `tyk-portal`) that import this package and interact directly with this field will fail to compile and will require updates.\
- **Work-in-Progress**: The code is not ready for merge. The combination of known-failing tests, `//todo` comments acknowledging the flaws, and the inconsistent application of the new immutable pattern confirms that this is an incomplete solution that does not yet fix the target bug.",
  "tags": {
    "review-effort": 4,
    "label": "bug"
  }
}
<!-- visor:section-end id="overview" -->

<!-- visor:thread-end key="TykTechnologies/tyk#7530@65bd416" -->

---

*Powered by [Visor](https://probelabs.com/visor) from [Probelabs](https://probelabs.com)*

*Last updated: 2025-11-27T08:27:01.499Z | Triggered by: pr_updated | Commit: 65bd416*

💡 **TIP:** You can chat with Visor using `/visor ask <your question>`
<!-- /visor-comment-id:visor-thread-overview-TykTechnologies/tyk#7530 -->

[probelabs]

Security Issues (2)

Severity	Location	Issue
🟠 Error	apidef/oas/security.go:81	The `SecuritySchemes` struct is designed to be immutable, where methods like `delete` return a new, modified instance. The code calls `s.getTykSecuritySchemes().delete(...)` but ignores the returned new instance. As a result, security schemes that are supposed to be omitted from the configuration are not removed. This could lead to a security misconfiguration where an auth method believed to be disabled remains active with default (and potentially insecure) settings. The same issue exists on lines 395 and 514. 💡 Suggestion Create a helper method on `OAS` to handle deletion from `SecuritySchemes` and reassign the result, similar to `setTykSecurityScheme`. For example: ```go func (s *OAS) deleteTykSecurityScheme(key string) { auth := s.getTykAuthentication() if auth != nil { auth.SecuritySchemes = auth.SecuritySchemes.delete(key) } } ``` Then call this helper where needed, e.g., `s.deleteTykSecurityScheme(authConfig.Name)`.
🟠 Error	apidef/oas/authentication.go:228-230	The function `decodeScheme` is a stub that always returns a "not implemented" error. This function is called during JSON and YAML unmarshalling of `SecuritySchemes`. Consequently, any attempt to deserialize a configuration containing security schemes will fail. This is a denial-of-service vulnerability, as it will prevent the gateway from loading valid API definitions. 💡 Suggestion Implement the `decodeScheme` function to correctly identify the type of security scheme from the raw `interface{}` data (likely a `map[string]interface{}`) and decode it into the corresponding struct (e.g., `*Token`, `*JWT`, etc.). This will likely require inspecting fields to determine the scheme type, similar to the logic in the `importNative` function.

Architecture Issues (5)

Severity	Location	Issue
🔴 Critical	apidef/oas/authentication.go:39-51	The refactoring of `SecuritySchemes` into a struct fails to address the original race condition. The new struct lacks any synchronization primitives (like a `sync.RWMutex`), and operations on it are not atomic. Concurrent access to a shared `OAS` object will still lead to race conditions like lost updates, and potentially the original `concurrent map writes` panic if the internal map is accessed concurrently. The PR's primary goal of fixing the concurrency issue is not met. 💡 Suggestion To ensure thread safety, add a `sync.RWMutex` to the `SecuritySchemes` struct. Protect all reads and writes to the internal `container` map with the mutex. This typically involves changing methods to use pointer receivers (`func (ss *SecuritySchemes) ...`) and modifying the map in-place under lock protection, which is a more idiomatic and robust solution for concurrent map access in Go.
🔴 Critical	apidef/oas/authentication.go:227-229	The `decodeScheme` function, which is critical for deserializing security schemes from JSON or YAML, is not implemented and returns a hardcoded 'not implemented' error. This makes the new `SecuritySchemes` struct unusable for any configuration that needs to be loaded from a file or external source, as unmarshalling will always fail. 💡 Suggestion Implement the `decodeScheme` function. It should inspect the raw `interface{}` data to determine the concrete type of the security scheme (e.g., by checking for fields unique to JWT, Basic, Token, etc.) and then correctly unmarshal the data into the corresponding struct type.
🟠 Error	apidef/oas/security.go:75	The immutable value object pattern for `SecuritySchemes` is used incorrectly. The `delete` method returns a new instance with the item removed, but its return value is ignored in multiple places (e.g., `fillToken`, `fillJWT`, `fillBasic`). This results in a bug where deletions have no effect, leaving stale security schemes in the configuration. 💡 Suggestion Ensure that the return value of `delete` is always reassigned back to the `SecuritySchemes` field on the `Authentication` struct. For example: `auth.SecuritySchemes = auth.SecuritySchemes.delete(name)`. To improve encapsulation and reduce misuse, consider adding a `deleteTykSecurityScheme` method to the `*OAS` type, similar to the existing `setTykSecurityScheme`.
🟡 Warning	apidef/oas/authentication.go:39-105	The chosen immutable value object pattern for `SecuritySchemes` introduces unnecessary complexity and has proven to be error-prone, as seen by the incorrect usage of the `delete` method. This pattern, which involves cloning the map on every mutation, also has potential performance overhead. A simpler, more idiomatic Go approach using a mutex-protected struct would be easier to understand, less susceptible to misuse, and would have correctly solved the underlying concurrency problem. 💡 Suggestion Re-evaluate the architectural decision to use an immutable data structure. Refactor `SecuritySchemes` to use a standard Go pattern for concurrent-safe maps: a struct containing the map and a `sync.RWMutex`. This would simplify the implementation and eliminate the need for callers to manage state by reassigning return values.
🟡 Warning	apidef/oas/oas.go:224-226	The comment for the `getTypedSchema` function is highly misleading. It claims the function performs atomic promotion and caching using `RLock`/`Lock`, but the implementation contains no synchronization primitives, atomic operations, or caching logic. This suggests the implementation is incomplete and misrepresents its behavior. 💡 Suggestion Update the comment to accurately describe what the function does (a simple get and type assertion) or implement the described atomic/caching behavior. Given the lack of thread safety in the `SecuritySchemes` struct, implementing this behavior would require significant changes.

Performance Issues (2)

Severity	Location	Issue
🟢 Info	apidef/oas/authentication.go:247-251	The `Set` method performs a deep clone of the entire security schemes map on every call. When importing multiple security schemes in a loop (as done in `importAuthentication`), this results in repeatedly cloning a growing map, leading to quadratic complexity (O(n²)) in terms of elements copied. While the number of security schemes is typically small, this can become a performance bottleneck during API definition loading for APIs with many security schemes. 💡 Suggestion For bulk updates, such as in `importAuthentication`, consider using a builder pattern or creating a new map and setting it once at the end to avoid repeated cloning. Alternatively, if performance during API loading becomes a concern, replacing the immutability pattern with a `sync.RWMutex` to protect a mutable map would be more performant for writes.
🟢 Info	apidef/oas/authentication.go:276	The `delete` method performs a deep clone of the entire security schemes map on every call. This is inefficient if multiple deletions are needed, as it repeatedly copies the map. 💡 Suggestion If bulk deletions are a possibility, provide a method to remove multiple keys at once to reduce the number of cloning operations. As with the `Set` method, if API loading performance is critical, consider using a mutex-guarded mutable map instead of an immutable one.

Quality Issues (4)

Severity	Location	Issue
🔴 Critical	apidef/oas/authentication.go:32-216	The refactoring of `SecuritySchemes` from a map to a struct does not solve the underlying concurrency issue. The new struct lacks synchronization primitives (e.g., a `sync.RWMutex`), making it unsafe for concurrent access. The original problem was a 'concurrent map writes' panic, and this implementation is still susceptible to race conditions when an object containing `SecuritySchemes` is modified by multiple goroutines. 💡 Suggestion To ensure thread safety, add a `sync.RWMutex` to the `SecuritySchemes` struct. Use a write lock (`mu.Lock()`) in methods that modify the internal map (like `Set` and `delete`) and a read lock (`mu.RLock()`) in methods that only read from it (like `Get`, `Len`, `Iter`). Consider using pointer receivers for mutating methods to modify the struct in-place, which is a more common and less error-prone pattern for stateful objects in Go than the immutable approach attempted here.
🔴 Critical	apidef/oas/authentication.go:214-216	The function `decodeScheme` is not implemented and returns a hardcoded error. This function is essential for `UnmarshalJSON` and `UnmarshalYAML` to work, as it's responsible for converting a raw map into a specific security scheme struct. As a result, any attempt to deserialize an API definition with security schemes from JSON or YAML will fail. 💡 Suggestion Implement the `decodeScheme` function. It should inspect the input `map[string]interface{}` to determine the type of security scheme (e.g., by checking keys like `type`, `scheme`, `bearerFormat`) and then use a library like `mapstructure` to decode the map into the corresponding struct pointer (e.g., `*JWT`, `*Token`).
🟠 Error	apidef/oas/oas.go:224-235	The new `getTypedSchema` function only performs a type assertion and no longer attempts to convert a `map[string]interface{}` to a typed struct. This is a regression from the previous `toStructIfMap` logic. If a security scheme is loaded from a configuration file, it will exist as a raw map, and this function will fail to return a typed struct, instead returning a zero value. This will likely lead to nil pointer panics downstream. 💡 Suggestion Update `getTypedSchema` to handle the case where the retrieved scheme is a `map[string]interface{}`. If the type assertion fails, use `mapstructure` to decode the map into the target generic type `T`, similar to how `loadForImport` is implemented. This would make the behavior consistent and robust.
🟠 Error	apidef/oas/authentication_test.go:494	The pull request includes multiple new tests that are explicitly marked with comments like `// TODO: Fails because...`. This indicates the code is a work-in-progress and not ready for merging. Shipping code with known failing tests that highlight fundamental design flaws (like state not being persisted correctly) is a significant quality risk. 💡 Suggestion Ensure all tests pass and remove the `TODO` comments related to failing tests before marking the pull request as ready for review. The failing tests in `apidef/oas/authentication_test.go` and `apidef/oas/oas_test.go` must be fixed as they point to critical issues in the implementation.

---

Powered by Visor from Probelabs

Last updated: 2025-11-27T08:27:04.471Z | Triggered by: pr_updated | Commit: 65bd416

💡 TIP: You can chat with Visor using /visor ask <your question>

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[andrei-tyk]

This PR has been closed in favour of #7579.