Skip to content
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

DR-112 - New Feature #29

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 about 2 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.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@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
Copilot AI reviewed Nov 14, 2024
View reviewed changes
Copy link

@Copilot 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.

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 suggestions.

Files not reviewed (1)
  • pom.xml: Language not supported

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

src/main/java/net/codejava/SalesDAO.java
e.printStackTrace(); // log any other exceptions
}
public void save(Sale sale) {
String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")";
Copy link
Preview

Copilot AI Nov 14, 2024

Choose a reason for hiding this comment

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

The new save method introduces a potential SQL injection vulnerability. Use parameterized queries to prevent SQL injection.

Suggested change
String sql = "INSERT INTO SALES (item, quantity, amount) VALUES ('" + sale.getItem() + "', " + sale.getQuantity() + ", " + sale.getAmount() + ")";
String sql = "INSERT INTO SALES (item, quantity, amount) VALUES (?, ?, ?)";
jdbcTemplate.update(sql, sale.getItem(), sale.getQuantity(), sale.getAmount());

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/main/java/net/codejava/SalesDAO.java
} catch (Exception e) {
e.printStackTrace(); // log any other exceptions
}
public void save(Sale sale) {
Copy link
Preview

Copilot AI Nov 14, 2024

Choose a reason for hiding this comment

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

The removal of the null check for the sale object might lead to a NullPointerException. Re-add the null check for the sale object.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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.
1 participant
@tsviz