Skip to content

DR-112 - New Feature #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

DR-112 - New Feature #29

wants to merge 5 commits into from

Conversation

@tsviz
Copy link
Contributor

@tsviz tsviz commented Mar 22, 2024 •
edited
Loading

This pull request includes changes to the GitHub Actions workflow file .github/workflows/ci.yml, pom.xml, src/main/java/net/codejava/SalesDAO.java, and src/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 in SalesDAO.java. Additionally, a new dependency was added to pom.xml and the color scheme in styles.js was updated.
CI Workflow modifications:

Addition of a new dependency:

  • pom.xml: Added a new dependency for spring-security-core.

Changes in the SalesDAO.java file:

Changes in the styles.js file:

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 22, 2024
View reviewed changes
src/main/java/net/codejava/SalesDAO.java
}
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 High

This query depends on a
user-provided value
Loading
.

Copilot Autofix

AI 12 months 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 save method to use placeholders (?) for the values.
  • Use jdbcTemplate.update with the SQL query and the values from the Sale object as parameters.
Suggested changeset 1
src/main/java/net/codejava/SalesDAO.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/main/java/net/codejava/SalesDAO.java b/src/main/java/net/codejava/SalesDAO.java
--- a/src/main/java/net/codejava/SalesDAO.java
+++ b/src/main/java/net/codejava/SalesDAO.java
@@ -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());
+	}
 
EOF
@@ -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());
}

Copilot is powered by AI and may make mistakes. Always verify output.
@tsviz tsviz closed this May 13, 2024
@tsviz tsviz deleted the DR-112 branch May 13, 2024 16:02
@tsviz tsviz restored the DR-112 branch May 13, 2024 16:06
@tsviz tsviz reopened this May 13, 2024
@tsviz tsviz requested a review from Copilot November 14, 2024 16:50
Copilot

This comment was marked as outdated.

Sign in to view

@imanmahjoubi imanmahjoubi requested a review from Copilot June 10, 2025 21:07
Copilot
Copilot AI reviewed Jun 10, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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): renamed ssh_debug_enabled to debug_enabled, added javascript to CodeQL matrix, downgraded CodeQL actions to v2, updated test splitting glob, and removed the publish-test-results job.
  • Maven (pom.xml): added spring-security-core dependency.
  • Java (SalesDAO.java): replaced validation/insert logic with a raw SQL string.
  • JavaScript (styles.js): swapped theme check and made --h1-color configurable via window.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/**/**/**.java is overly specific and may skip tests; using src/test/**/*.java would reliably include all Java test files.
glob: src/test/**/**/**.java

src/main/java/net/codejava/SalesDAO.java:34

  • By removing the serial_number and date checks 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-core version 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>

src/main/java/net/codejava/SalesDAO.java
Comment on lines +33 to 36
public void save(Sale sale) {
String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")";
jdbcTemplate.update(sql);
}
Copy link

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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());
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tsviz