SONARJAVA-6528 S2068, S6418, and S6437: Make use of common secret exclusion filter by pierre-loup-tristant-sonarsource · Pull Request #5745 · SonarSource/sonar-java

pierre-loup-tristant-sonarsource · 2026-07-02T14:41:36Z

Summary by Gitar

Dependency updates:
- Upgraded analyzer-commons to version 2.25.0.4954.
Security checks improvements:
- Integrated SecretClassifier into S2068, S6418, and S6437 to filter out known non-secret fake values.
- Refactored AbstractHardCodedCredentialChecker to use SecretClassifier for identifying potential credentials instead of hardcoded lists and length checks.

_{This will update automatically on new commits.}

hashicorp-vault-sonar-prod · 2026-07-02T14:43:15Z

gitar-bot · 2026-07-02T15:25:47Z

+    String variable6 = "login=a&bazooka=xxx"; // Compliant, short value filter
+    String variable6_2 = "login=a&bazooka=xvxf6_gaa"; // Noncompliant
+
+    String variableNameWithBazookaInIt = "xxx"; // Compliant, , short value filter


💡 Quality: Typos in new test-fixture comments

A few of the newly added/modified explanatory comments contain typos:

HardCodedPasswordCheckCustom.java:20 and :28 both read // Compliant, , short value filter with a doubled comma.

HardCodedPasswordCheckSample.java:226 uses a triple-slash /// instead of // (myA.setProperty("password", "xxxxx"); /// Compliant, short value filter).

These are harmless to the check semantics (comments only) but worth cleaning up for consistency with the surrounding fixtures. Fix by removing the extra comma and the extra slash respectively.

_{Was this helpful? React with 👍 / 👎}

gitar-bot · 2026-07-06T07:24:06Z

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Refactors S2068, S6418, and S6437 to utilize the common secret exclusion filter. Address minor typos identified in the new test-fixture comments to finalize the implementation.

💡 Quality: Typos in new test-fixture comments

📄 java-checks-test-sources/default/src/main/java/checks/HardCodedPasswordCheckCustom.java:20 📄 java-checks-test-sources/default/src/main/java/checks/HardCodedPasswordCheckCustom.java:28 📄 java-checks-test-sources/default/src/main/java/checks/HardCodedPasswordCheckSample.java:226 📄 java-checks-test-sources/aws/src/main/java/checks/security/HardCodedCredentialsShouldNotBeUsedCheckSample.java:146

A few of the newly added/modified explanatory comments contain typos:

HardCodedPasswordCheckCustom.java:20 and :28 both read // Compliant, , short value filter with a doubled comma.
HardCodedPasswordCheckSample.java:226 uses a triple-slash /// instead of // (myA.setProperty("password", "xxxxx"); /// Compliant, short value filter).

These are harmless to the check semantics (comments only) but worth cleaning up for consistency with the surrounding fixtures. Fix by removing the extra comma and the extra slash respectively.

🤖 Prompt for agents

Code Review: Refactors S2068, S6418, and S6437 to utilize the common secret exclusion filter. Address minor typos identified in the new test-fixture comments to finalize the implementation.

1. 💡 Quality: Typos in new test-fixture comments
   Files: java-checks-test-sources/default/src/main/java/checks/HardCodedPasswordCheckCustom.java:20, java-checks-test-sources/default/src/main/java/checks/HardCodedPasswordCheckCustom.java:28, java-checks-test-sources/default/src/main/java/checks/HardCodedPasswordCheckSample.java:226, java-checks-test-sources/aws/src/main/java/checks/security/HardCodedCredentialsShouldNotBeUsedCheckSample.java:146

   A few of the newly added/modified explanatory comments contain typos:
   - `HardCodedPasswordCheckCustom.java:20` and `:28` both read `// Compliant, , short value filter` with a doubled comma.
   - `HardCodedPasswordCheckSample.java:226` uses a triple-slash `///` instead of `//` (`myA.setProperty("password", "xxxxx"); /// Compliant, short value filter`).
   
   These are harmless to the check semantics (comments only) but worth cleaning up for consistency with the surrounding fixtures. Fix by removing the extra comma and the extra slash respectively.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

sonarqube-next · 2026-07-06T07:30:32Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

asya-vorobeva

💯

teemu-rytilahti-sonarsource

Thank you @pierre-loup-tristant-sonarsource, looks great already! I made some suggestions inline on how to improve this, let me know if something feels off and/or unclear!

teemu-rytilahti-sonarsource · 2026-07-07T08:43:31Z

+  static final String FINAL_SECRET_STRING = "hunter2"; // Compliant, fake value filter
+  static final String FINAL_SECRET_STRING_2 = "xvxf6_gaa";


Should one of the names indicate that it's a fake? Alas, that would balloon the diff :-(

teemu-rytilahti-sonarsource · 2026-07-07T17:11:08Z

    SHA256.getHMAC((FINAL_SECRET_STRING).getBytes("UTF-8"), message); // Noncompliant
-    SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant
+    // FP should be filtered as short string value
+    SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant 


Suggested change

SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant

SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant

This is about a hardcoded key for getHMAC, we probably should not suppress it?

teemu-rytilahti-sonarsource · 2026-07-07T17:14:26Z

+//                            ^^^^^^^^^^^^^^^^^^^^^^>
+    request.login("user", plainTextSecret2); // Noncompliant
+//                        ^^^^^^^^^^^^^^^^
    request.login("user", new String("secret")); // Noncompliant


Was this not already suppressed, even before SonarSource/sonar-analyzer-commons#418?

teemu-rytilahti-sonarsource · 2026-07-07T17:15:45Z

+    // TP
+    Encryptors.delux("xvxf6_gaa".subSequence(0, 0), "salt"); // Noncompliant


Suggested change

// TP

Encryptors.delux("xvxf6_gaa".subSequence(0, 0), "salt"); // Noncompliant

Encryptors.delux("xvxf6_gaa".subSequence(0, 0), "salt"); // Noncompliant

teemu-rytilahti-sonarsource · 2026-07-07T17:17:00Z

+    // "password".subSequence(0, 0) cannot be resolved to the value "password" that should be filtered -> FPs
    Encryptors.delux("password".subSequence(0, 0), "salt"); // Noncompliant


Suggested change

// "password".subSequence(0, 0) cannot be resolved to the value "password" that should be filtered -> FPs

Encryptors.delux("password".subSequence(0, 0), "salt"); // Noncompliant

Encryptors.delux("password".subSequence(0, 0), "salt"); // Noncompliant, FP, subSequence is not resolved to a filtered "password" value.

I would follow the style where the comment for the marker is on the same line, but worth confirming how it is done elsewhere in the project.

teemu-rytilahti-sonarsource · 2026-07-07T17:34:30Z

    String okApiKeyValue = "Spaces are UNEXPECTED 012 345 678"; // Compliant
    String tokenism = "(Queen's Partner's Stored Knowledge is a Minimal Sham)"; // Compliant
-    String tokenWithExcludedCharacters2 = "abcdefghij|klmnopqrs"; // Compliant
+    String tokenWithExcludedCharacters2 = "bacdefghij|klmnopqrs"; // Noncompliant


Keep the original with a note why it's now compliant.

teemu-rytilahti-sonarsource · 2026-07-07T17:36:24Z

    if("password".equals(auth)) {
    }
-    if(auth.equals("password-1234")) {
+    if(auth.equals("password-2134")) {


I think keeping the original should also work.

teemu-rytilahti-sonarsource · 2026-07-07T17:36:47Z

+    myA.setProperty("api-key", "bacdefghijklmnopqrs"); // Noncompliant
+    myA.setProperty("okapi-keyboard", "bacdefghijklmnopqrs"); // Compliant
    myA.setProperty("secret", "X");
    myA.setProperty("secret", "anonymous");


Should raise now?

teemu-rytilahti-sonarsource · 2026-07-07T17:39:32Z

-      || followingString.startsWith("?")
-      || followingString.startsWith(":")
-      || followingString.contains("%s");


Given how many different checks this does, it has probably been tuned on purpose to exclude those sql named parameter looking strings, and I would be a bit hesitant to change the logic here. Maybe it's a good idea to do so though to keep it consistent, but perhaps we should still keep the anonymous case by moving it to the commons?

teemu-rytilahti-sonarsource · 2026-07-07T17:41:09Z


-  private static boolean isURLWithCredentials(String stringLiteral) {
-    if (URL_PREFIX.matcher(stringLiteral).find()) {
+  private static Optional<String> extractURLPassword(String url) {


Any specific reason to use Optional instead of keeping the original form?

pierre-loup-tristant-sonarsource force-pushed the plt/sonarjava-6528 branch from da498a5 to 7dc970a Compare July 2, 2026 15:24

gitar-bot Bot reviewed Jul 2, 2026

View reviewed changes

pierre-loup-tristant-sonarsource force-pushed the plt/sonarjava-6528 branch from 7dc970a to 8067e70 Compare July 3, 2026 13:52

S2068, S6418, and S6437: Make use of common secret exclusion filter

d6658b5

pierre-loup-tristant-sonarsource force-pushed the plt/sonarjava-6528 branch from 8067e70 to d6658b5 Compare July 6, 2026 07:23

asya-vorobeva approved these changes Jul 7, 2026

View reviewed changes

teemu-rytilahti-sonarsource requested changes Jul 7, 2026

View reviewed changes

		static final String FINAL_SECRET_STRING = "hunter2"; // Compliant, fake value filter
		static final String FINAL_SECRET_STRING_2 = "xvxf6_gaa";

	SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant
	SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant

		// TP
		Encryptors.delux("xvxf6_gaa".subSequence(0, 0), "salt"); // Noncompliant

		// "password".subSequence(0, 0) cannot be resolved to the value "password" that should be filtered -> FPs
		Encryptors.delux("password".subSequence(0, 0), "salt"); // Noncompliant

Uh oh!

Conversation

pierre-loup-tristant-sonarsource commented Jul 2, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Gitar

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitar-bot Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Jul 6, 2026

Uh oh!

sonarqube-next Bot commented Jul 6, 2026

Quality Gate passed

Uh oh!

asya-vorobeva left a comment

Choose a reason for hiding this comment

Uh oh!

teemu-rytilahti-sonarsource left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pierre-loup-tristant-sonarsource commented Jul 2, 2026 •

edited by gitar-bot Bot

Loading

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 •

edited by atlassian Bot

Loading