[TT-16013] Gateway panics when validating JWT claims #7530
Closed
probelabs / Visor: quality
failed
Nov 27, 2025 in 1m 25s
🚨 Check Failed
quality check failed because fail_if condition was met.
Details
📊 Summary
- Total Issues: 5
- Critical Issues: 2
- Error Issues: 3
🔍 Failure Condition Results
Failed Conditions
- global_fail_if: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')
- Severity: ❌ error
Issues by Category
Architecture (1)
- 🚨 apidef/oas/authentication.go:32 - The refactoring of
SecuritySchemesfrom a map to a struct does not solve the underlying concurrency issue. The new struct lacks synchronization primitives (e.g., async.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 containingSecuritySchemesis modified by multiple goroutines.
Logic (3)
- 🚨 apidef/oas/authentication.go:214 - The function
decodeSchemeis not implemented and returns a hardcoded error. This function is essential forUnmarshalJSONandUnmarshalYAMLto 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. - ❌ apidef/oas/oas.go:224 - The new
getTypedSchemafunction only performs a type assertion and no longer attempts to convert amap[string]interface{}to a typed struct. This is a regression from the previoustoStructIfMaplogic. 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. - ❌ system:0 - Global failure condition met: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')
Testing (1)
- ❌ 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.
Powered by Visor from Probelabs
💡 TIP: You can chat with Visor using /visor ask <your question>
Annotations
Check failure on line 216 in apidef/oas/authentication.go
probelabs / Visor: quality
architecture Issue
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.
Raw output
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.
Check failure on line 216 in apidef/oas/authentication.go
probelabs / Visor: quality
logic Issue
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.
Raw output
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`).
Check failure on line 235 in apidef/oas/oas.go
probelabs / Visor: quality
logic Issue
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.Raw output
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.
Check failure on line 494 in apidef/oas/authentication_test.go
probelabs / Visor: quality
testing Issue
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.
Raw output
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.
Loading