fix: Security audit — fix critical vulnerabilities in API specification#3
Open
devin-ai-integration[bot] wants to merge 1 commit into
Open
fix: Security audit — fix critical vulnerabilities in API specification#3devin-ai-integration[bot] wants to merge 1 commit into
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
- Add JWT Bearer authentication scheme (was completely missing) - Split UserModel into separate request/response schemas to prevent password leakage in API responses - Add authentication requirements to all sensitive endpoints (GetAvailableFunds, ProfileImage, PortalSearch) - Remove IDOR-susceptible user query params from own-data endpoints - Add input validation constraints (minLength, maxLength, minimum, maximum, pattern) to all request fields - Remove client-controlled price from BuyProductReq - Change Logout from GET to POST (CSRF prevention) - Add 429 rate limiting responses to auth endpoints - Fix user enumeration in PasswordRecovery - Add 403 Forbidden responses to admin endpoints - Move OpenAPI spec from README.md to openapi.json - Add SECURITY.md documenting all findings and fixes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The repo contained only an OpenAPI spec (misplaced in README.md) for "BankWeb API" with 12 security vulnerabilities across critical/high/medium severity. Key fixes:
Critical:
components.securitySchemes.BearerAuth(JWT) — spec had zero auth mechanism definedUserModel(which includedpassword) intoLoginRequest/LoginResponse/RegisterRequestetc. — password was returned in Login responsesecurity: [{BearerAuth: []}]+401to/api/User/GetAvailableFunds,/api/User/ProfileImage,/api/PortalSearch/Index— these were completely unauthenticatedHigh:
?username=/?user=query params from own-data endpoints (GetHistory,GetTransactionHistory,GetAvailableFunds,ProfileImage) — user must be derived from JWT token server-sideminLength/maxLength/minimum/maximum/patternto all request fields — previously unbounded, enabling injection and DoSpricefromBuyProductReq— client could set arbitrary purchase priceMedium:
/api/Auth/Logoutfrom GET → POST (CSRF via<img>tag)429responses to auth endpoints (brute-force protection)/api/Auth/PasswordRecovery(always 200)Also: moved spec from README.md →
openapi.json, added properREADME.mdandSECURITY.mddocumenting all findings + server-side implementation recommendations (CORS, SQLi prevention, password hashing, token management).Link to Devin session: https://app.devin.ai/sessions/367c97de0e044f39bf3c245d89441087
Requested by: @wsh92