Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| public void save(Sale sale) { | ||
| String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")"; | ||
| jdbcTemplate.update(sql); |
Check failure
Code scanning / CodeQL
Query built from user-controlled sources
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI over 1 year ago
To fix the problem, we need to replace the string concatenation in the save method with a parameterized query using PreparedStatement. This will ensure that user input is properly escaped and prevent SQL injection attacks.
- Change the SQL query construction in the
savemethod to use placeholders (?) for the values. - Use
jdbcTemplate.updatewith the SQL query and the values from theSaleobject as parameters.
| @@ -32,6 +32,6 @@ | ||
|
|
||
| public void save(Sale sale) { | ||
| String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")"; | ||
| jdbcTemplate.update(sql); | ||
| } | ||
| public void save(Sale sale) { | ||
| String sql = "INSERT INTO SALES (item, quantity, amount) VALUES (?, ?, ?)"; | ||
| jdbcTemplate.update(sql, sale.getItem(), sale.getQuantity(), sale.getAmount()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the CI workflow inputs and tooling versions, adds JavaScript analysis, adjusts test splitting, and removes the publish-test-results job; introduces a new Spring Security dependency; simplifies the SalesDAO.save implementation; and makes header colors in the JS styles configurable.
- CI Workflow (
.github/workflows/ci.yml): renamedssh_debug_enabledtodebug_enabled, addedjavascriptto CodeQL matrix, downgraded CodeQL actions to v2, updated test splitting glob, and removed the publish-test-results job. - Maven (
pom.xml): addedspring-security-coredependency. - Java (
SalesDAO.java): replaced validation/insert logic with a raw SQL string. - JavaScript (
styles.js): swapped theme check and made--h1-colorconfigurable viawindow.searchFeatureColor.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/ci.yml | CI inputs renamed, CodeQL downgraded, JS added, test glob updated, publish-tests removed |
| pom.xml | Added spring-security-core dependency |
| src/main/java/net/codejava/SalesDAO.java | Simplified save to raw SQL update |
| src/main/resources/static/js/styles.js | Made header color dynamic with window.searchFeatureColor |
Comments suppressed due to low confidence (3)
.github/workflows/ci.yml:238
- The glob
src/test/**/**/**.javais overly specific and may skip tests; usingsrc/test/**/*.javawould reliably include all Java test files.
glob: src/test/**/**/**.java
src/main/java/net/codejava/SalesDAO.java:34
- By removing the
serial_numberanddatechecks and columns, the DAO no longer enforces required fields and uniqueness; consider restoring those validations or handling them upstream.
String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")";
pom.xml:142
- [nitpick] Specifying a fixed
spring-security-coreversion may conflict with the project's Spring Boot BOM; consider inheriting the version from the parent or a dependency management section.
<version>5.7.0</version>
| public void save(Sale sale) { | ||
| String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")"; | ||
| jdbcTemplate.update(sql); | ||
| } |
There was a problem hiding this comment.
This concatenated SQL string is vulnerable to SQL injection and omits the serial_number and date columns; switch to a parameterized query using jdbcTemplate.update(String, Object...) or NamedParameterJdbcTemplate.
| public void save(Sale sale) { | |
| String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")"; | |
| jdbcTemplate.update(sql); | |
| } | |
| public void save(Sale sale) { | |
| String sql = "INSERT INTO SALES (item, quantity, amount) VALUES (?, ?, ?)"; | |
| jdbcTemplate.update(sql, sale.getItem(), sale.getQuantity(), sale.getAmount()); | |
| } |
1 participant
This pull request includes changes to the GitHub Actions workflow file
.github/workflows/ci.yml,pom.xml,src/main/java/net/codejava/SalesDAO.java, andsrc/main/resources/static/js/styles.js. The changes mainly involve the renaming and simplification of debugging steps, addition of JavaScript as a language in the CodeQL analysis, downgrading of CodeQL and Autobuild actions, modification of the test splitting glob pattern, removal of the publish-test-results job, and changes in the save method inSalesDAO.java. Additionally, a new dependency was added topom.xmland the color scheme instyles.jswas updated.CI Workflow modifications:
.github/workflows/ci.yml: Renamed thessh_debug_enabledinput todebug_enabledand updated its description..github/workflows/ci.yml: Added 'javascript' to thelanguagematrix..github/workflows/ci.yml: Downgraded the version ofgithub/codeql-action/init,github/codeql-action/autobuild, andgithub/codeql-action/analyzefrom v3 to v2. [1] [2].github/workflows/ci.yml: Updated theifcondition in theSetup tmate sessionstep to use the newdebug_enabledinput..github/workflows/ci.yml: Removed thepublish-test-resultsjob..github/workflows/ci.yml: Removed thedebuginput from thewithsection of thedeploy-to-awsjob.Addition of a new dependency:
pom.xml: Added a new dependency forspring-security-core.Changes in the
SalesDAO.javafile:src/main/java/net/codejava/SalesDAO.java: Simplified thesavemethod by removing the checks for duplicate keys and null values, and changed the SQL statement.Changes in the
styles.jsfile:src/main/resources/static/js/styles.js: Changed the color of headers when the search feature is enabled.