fix: prevent nil pointer panic on invalid X-API-Key in admin endpoints#4881
fix: prevent nil pointer panic on invalid X-API-Key in admin endpoints#4881kodiakhq[bot] merged 5 commits intomainfrom
Conversation
When ErrorHandler is nil in KeyAuthExtendedConfig (which happens when using echo's DefaultKeyAuthConfig defaults), sending an invalid X-API-Key causes a nil pointer dereference panic instead of returning 401. This adds nil checks for ErrorHandler in validateXAPIKey and explicitly sets AuthMiddlewareErrHandler on the admin route group config. Closes #4842 Made-with: Cursor
✅ Deploy Preview for go-feature-flag-doc-preview canceled.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request effectively addresses a nil pointer dereference issue in the KeyAuthExtended middleware when an invalid X-API-Key is provided and no custom ErrorHandler is configured. The fix involves adding nil checks before invoking the ErrorHandler and explicitly setting a custom error handler for admin routes to ensure consistent behavior. New tests have been added to reproduce and verify the fix.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4881 +/- ##
==========================================
+ Coverage 85.75% 85.81% +0.05%
==========================================
Files 153 153
Lines 6537 6541 +4
==========================================
+ Hits 5606 5613 +7
+ Misses 699 698 -1
+ Partials 232 230 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add tests for the case where the validator returns a non-nil error (e.g. a database failure), exercising both the nil ErrorHandler guard (line 65) and the ErrorHandler delegation (line 67). This brings patch coverage to 100% for the changes in #4842. Made-with: Cursor
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
|
1 participant
Description
When accessing admin endpoints (e.g.
POST /admin/v1/retriever/refresh) with an invalidX-API-Key, the relay proxy panics with a nil pointer dereference instead of returning401 Unauthorized.The root cause is that
ErrorHandlerinKeyAuthExtendedConfigdefaults tonil(copied from echo'sDefaultKeyAuthConfig.ErrorHandlerwhich is alsonil). WhenvalidateXAPIKeyencounters an invalid key, it callsconfig.ErrorHandler(...)without checking fornilfirst.Fix:
validateXAPIKeyso that ifErrorHandleris nil, the error is returned directly instead of panicking.ErrorHandler: AuthMiddlewareErrHandleron the admin route group config to ensure consistent error handling.ErrorHandler.Closes issue(s)
Resolve #4842
Checklist
README.mdand/website/docs)Made with Cursor