fix: remediate 8 security vulnerabilities (CSRF, amount bypass, @Transactional, input validation, headers, dependency CVE)#177
Open
devin-ai-integration[bot] wants to merge 3 commits intoDevOpsfrom
Conversation
C1: Re-enable CSRF protection (remove csrf.disable()), update all HTML forms to use th:action for automatic CSRF token inclusion, convert logout to POST form C2: Add positive amount validation in deposit(), withdraw(), and transferAmount() to prevent negative amount bypass C3: Add @transactional to deposit(), withdraw(), and transferAmount() to ensure atomicity C4: Add username/password length validation in registerAccount() C6: Replace 'Username already exists' with generic error message to prevent username enumeration C8: Update mysql-connector-java 8.0.33 to mysql-connector-j 9.1.0 (new groupId com.mysql) C10: Add security headers (Content-Type-Options, HSTS) in SecurityConfig Build: Fix maven-compiler-plugin source/target from 1.8 to 17 Also adds H2 test dependency and test application.properties for in-memory database during tests. Co-Authored-By: Angela Lin <angela.lin@cognition.ai>
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:
|
Devin Review
|
…t error handling - Use username.trim().length() for consistent validation with trim-based emptiness check - Add try-catch in BankController.deposit() to handle amount validation errors gracefully instead of returning a raw 500 error page Co-Authored-By: Angela Lin <angela.lin@cognition.ai>
Co-Authored-By: Angela Lin <angela.lin@cognition.ai>
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
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
Implements targeted fixes for 8 confirmed/likely security vulnerabilities identified during security audit. All fixes are minimal, scoped, and verified to not break existing tests.
Findings Remediated
th:action, convert logout to POSTamount > 0validation in deposit/withdraw/transfer@Transactionalto deposit/withdraw/transfer methodsFiles Changed
SecurityConfig.java— Removedcsrf.disable(), added HSTS + content-type headers, changed logout to POST-onlyAccountService.java— Added@Transactional, positive amount validation, input length validation, generic registration errorpom.xml— Updated mysql-connector groupId/version, fixed compiler source/target to 17, added H2 test dependencylogin.html— Changed form action toth:action=\"@{/login}\"register.html— Changed form action toth:action=\"@{/register}\", display dynamic error messagedashboard.html— Changed all form actions toth:action, converted logout link to POST formtransactions.html— Converted logout link to POST formsrc/test/resources/application.properties— Added H2 in-memory DB config for testsRemediated: 8 | Not remediated: 0
Build status: pass | Test suite: pass (1 test, 0 failures)
Review & Testing Checklist for Human
_csrfhidden input). Test that submitting a form without a valid CSRF token is rejected (403)./logoutvia GET no longer logs out.com.mysql:mysql-connector-j:9.1.0connects properly.Notes
com.h2database:h2as a test-scoped dependency andsrc/test/resources/application.propertiesto enable the existing@SpringBootTestcontext test to run without a MySQL instance.AntPathRequestMatchernow requires POST method, matching the new form-based logout in the templates.devin/1777433096-security-remediationLink to Devin session: https://app.devin.ai/sessions/c59e301e861b46508257802ae123bf52
Requested by: @angelalincog