From af1304b21d46d0502a7e9e7ac21dbf3636373c1f Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Sun, 24 May 2026 04:48:13 +0530 Subject: [PATCH 1/4] test(java): add regression coverage for detection location (#339) Signed-off-by: Sachin Kumar --- .../issues/PreciseIssueLocationTestFile.java | 71 ++++++++++ .../issues/PreciseIssueLocationTest.java | 124 ++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 java/src/test/files/rules/issues/PreciseIssueLocationTestFile.java create mode 100644 java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java diff --git a/java/src/test/files/rules/issues/PreciseIssueLocationTestFile.java b/java/src/test/files/rules/issues/PreciseIssueLocationTestFile.java new file mode 100644 index 000000000..e9a013bfd --- /dev/null +++ b/java/src/test/files/rules/issues/PreciseIssueLocationTestFile.java @@ -0,0 +1,71 @@ +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import java.security.Key; + +/** + * Regression test for issue #339: Detection location should point to the actual + * call site, + * not to the closing '*''/' of the next method's javadoc comment. + * + *

+ * This file simulates the Guava Hashing.java pattern where methods that wrap a + * SecretKeySpec + * constructor are separated by javadoc comments. + */ +public class PreciseIssueLocationTestFile { + + /** + * Returns a hash function implementing HMAC-MD5 using the given byte array key. + * + * @param key the key material + */ + public static Mac hmacMd5(byte[] key) throws Exception { + SecretKeySpec spec = new SecretKeySpec(key, "HmacMD5"); // Noncompliant {{(SecretKey) HMAC}} + + return hmacMd5(spec); + } + + /** + * Returns a hash function implementing HMAC-MD5 using the given key. + * + * @param key the secret key + * @throws IllegalArgumentException if the given key is inappropriate + */ + public static Mac hmacMd5(Key key) throws Exception { + Mac mac = Mac.getInstance("HmacMD5"); // Noncompliant {{(Mac) HMAC-MD5}} + mac.init(key); + return mac; + } + + /** + * Returns a hash function implementing HMAC-SHA256 using the given byte array + * key. + * + *

+ * The returned hash function uses a {@link SecretKeySpec} created from the + * given byte array + * and the HmacSHA256 algorithm. + * + * @param key the key material of the secret key + * @since 20.0 + */ + public static Mac hmacSha256(byte[] key) throws Exception { + SecretKeySpec spec = new SecretKeySpec(key, "HmacSHA256"); // Noncompliant {{(SecretKey) HMAC}} + + return hmacSha256(spec); + } + + /** + * Returns a hash function implementing HMAC-SHA256 using the given key. + * + * @param key the secret key + * @throws IllegalArgumentException if the given key is inappropriate for + * initializing this MAC + * @since 20.0 + */ + public static Mac hmacSha256(Key key) throws Exception { + Mac mac = Mac.getInstance("HmacSHA256"); // Noncompliant {{(Mac) HMAC-SHA256}} + mac.init(key); + return mac; + } +} diff --git a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java new file mode 100644 index 000000000..4ccb90f8b --- /dev/null +++ b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java @@ -0,0 +1,124 @@ +/* + * Sonar Cryptography Plugin + * Copyright (C) 2024 PQCA + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.ibm.plugin.rules.issues; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.ibm.engine.detection.DetectionStore; +import com.ibm.engine.model.Algorithm; +import com.ibm.engine.model.IValue; +import com.ibm.engine.model.context.MacContext; +import com.ibm.engine.model.context.SecretKeyContext; +import com.ibm.mapper.model.INode; +import com.ibm.plugin.TestBase; +import java.util.List; +import javax.annotation.Nonnull; +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; +import org.sonar.plugins.java.api.JavaCheck; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.Tree; + +/** + * Regression test for issue #339: Detection location off - Findings reported below the actual + * place. + * + *

When scanning code that has methods separated by multi-line javadoc comments (like Guava's + * Hashing.java), all findings were reported at the closing {@code * /} of the next + * method's javadoc comment instead of at the actual detection site. + * + *

This test verifies that: + * + *

    + *
  1. Findings for {@code new SecretKeySpec(...)} are reported on the line of the constructor + * call, not on the javadoc comment of the following method. + *
  2. Findings for {@code Mac.getInstance(...)} are reported on the line of the method + * invocation, not on the javadoc comment of the following method. + *
+ * + *

The test fixture {@code PreciseIssueLocationTestFile.java} reproduces the Guava Hashing.java + * pattern exactly: each method is separated from the next by a multi-line {@code /** ... * /} + * javadoc block. + */ +// https://github.com/cbomkit/sonar-cryptography/issues/339 +class PreciseIssueLocationTest extends TestBase { + + /** + * Verifies that issues are reported at the correct lines (i.e., at the {@code //Noncompliant} + * markers in the test fixture, which are placed on the actual call-site lines, NOT on the + * closing {@code * /} of adjacent javadoc comments). + */ + @Test + void test() { + CheckVerifier.newVerifier() + .onFile("src/test/files/rules/issues/PreciseIssueLocationTestFile.java") + .withChecks(this) + .verifyIssues(); + } + + @Override + public void asserts( + int findingId, + @Nonnull DetectionStore detectionStore, + @Nonnull List nodes) { + switch (findingId) { + case 0 -> { + // new SecretKeySpec(key, "HmacMD5") in hmacMd5(byte[] key) + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(SecretKeyContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacMD5"); + } + case 1 -> { + // Mac.getInstance("HmacMD5") in hmacMd5(Key key) + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(MacContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacMD5"); + } + case 2 -> { + // new SecretKeySpec(key, "HmacSHA256") in hmacSha256(byte[] key) + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(SecretKeyContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacSHA256"); + } + case 3 -> { + // Mac.getInstance("HmacSHA256") in hmacSha256(Key key) + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(MacContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacSHA256"); + } + default -> { + // No additional findings expected + } + } + } +} From 9805b39a46587e62e4af91017daf548d3e38561e Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Sun, 24 May 2026 05:46:35 +0530 Subject: [PATCH 2/4] test(java): address review feedback for location regression Signed-off-by: Sachin Kumar --- .../issues/PreciseIssueLocationTest.java | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java index 4ccb90f8b..5f535be47 100644 --- a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java +++ b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java @@ -20,6 +20,7 @@ package com.ibm.plugin.rules.issues; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.fail; import com.ibm.engine.detection.DetectionStore; import com.ibm.engine.model.Algorithm; @@ -38,36 +39,48 @@ import org.sonar.plugins.java.api.tree.Tree; /** - * Regression test for issue #339: Detection location off - Findings reported below the actual + * Regression test for issue #339: Detection location off - Findings reported + * below the actual * place. * - *

When scanning code that has methods separated by multi-line javadoc comments (like Guava's - * Hashing.java), all findings were reported at the closing {@code * /} of the next + *

+ * When scanning code that has methods separated by multi-line javadoc comments + * (like Guava's + * Hashing.java), all findings were reported at the closing {@code * /} of the + * next * method's javadoc comment instead of at the actual detection site. * - *

This test verifies that: + *

+ * This test verifies that: * *

    - *
  1. Findings for {@code new SecretKeySpec(...)} are reported on the line of the constructor - * call, not on the javadoc comment of the following method. - *
  2. Findings for {@code Mac.getInstance(...)} are reported on the line of the method - * invocation, not on the javadoc comment of the following method. + *
  3. Findings for {@code new SecretKeySpec(...)} are reported on the line of + * the constructor + * call, not on the javadoc comment of the following method. + *
  4. Findings for {@code Mac.getInstance(...)} are reported on the line of the + * method + * invocation, not on the javadoc comment of the following method. *
* - *

The test fixture {@code PreciseIssueLocationTestFile.java} reproduces the Guava Hashing.java - * pattern exactly: each method is separated from the next by a multi-line {@code /** ... * /} + *

+ * The test fixture {@code PreciseIssueLocationTestFile.java} reproduces the + * Guava Hashing.java + * pattern exactly: each method is separated from the next by a multi-line + * {@code /** ... * /} * javadoc block. */ // https://github.com/cbomkit/sonar-cryptography/issues/339 class PreciseIssueLocationTest extends TestBase { /** - * Verifies that issues are reported at the correct lines (i.e., at the {@code //Noncompliant} - * markers in the test fixture, which are placed on the actual call-site lines, NOT on the + * Verifies that issues are reported at the correct lines (i.e., at the + * {@code //Noncompliant} + * markers in the test fixture, which are placed on the actual call-site lines, + * NOT on the * closing {@code * /} of adjacent javadoc comments). */ @Test - void test() { + void reportsIssuesOnCallSiteNotJavadoc() { CheckVerifier.newVerifier() .onFile("src/test/files/rules/issues/PreciseIssueLocationTestFile.java") .withChecks(this) @@ -116,9 +129,8 @@ public void asserts( assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacSHA256"); } - default -> { - // No additional findings expected - } + default -> fail( + "Unexpected findingId: " + findingId); } } } From 1aa02b94f1354d7de5acc8f69259680fcd3758be Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Sat, 30 May 2026 23:46:21 +0530 Subject: [PATCH 3/4] test(java): verify occurrence locations for issue #339 Signed-off-by: Sachin Kumar --- .../rules/issues/GuavaHashingTestFile.java | 139 ++++++++++++++++ .../plugin/rules/issues/GuavaHashingTest.java | 155 ++++++++++++++++++ .../issues/PreciseIssueLocationTest.java | 60 ++++--- 3 files changed, 328 insertions(+), 26 deletions(-) create mode 100644 java/src/test/files/rules/issues/GuavaHashingTestFile.java create mode 100644 java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java diff --git a/java/src/test/files/rules/issues/GuavaHashingTestFile.java b/java/src/test/files/rules/issues/GuavaHashingTestFile.java new file mode 100644 index 000000000..8d4204589 --- /dev/null +++ b/java/src/test/files/rules/issues/GuavaHashingTestFile.java @@ -0,0 +1,139 @@ +/* + * Sonar Cryptography Plugin + * Copyright (C) 2024 PQCA + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import java.security.Key; +import javax.crypto.spec.SecretKeySpec; + +/** + * Reproduction of the pkg:maven/com.google.guava/guava@33.0.0-jre Hashing.java pattern. + * + *

In the real Hashing.java (lines 285-379), each hmacXxx(byte[] key) method passes a nested + * {@code new SecretKeySpec(...)} directly as an argument to the overloaded hmacXxx(Key key) + * variant. Each pair is separated by a multi-line javadoc comment. + * + *

Actual source lines in guava-33.0.0-jre Hashing.java: + * + *

+ * + *

This fixture uses only JDK types but preserves the exact structural pattern: nested {@code new + * SecretKeySpec(...)} as a direct argument, methods separated by multi-line javadoc. + */ +public class GuavaHashingTestFile { + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the MD5 (128 hash bits) hash function and the given secret key. + * + * @param key the secret key + * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC + * @since 20.0 + */ + public static Key hmacMd5(Key key) { + return key; + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the MD5 (128 hash bits) hash function and a SecretKeySpec created from the given byte array + * and the MD5 algorithm. + * + * @param key the key material of the secret key + * @since 20.0 + */ + public static Key hmacMd5(byte[] key) { + return hmacMd5(new SecretKeySpec(key, "HmacMD5")); // Noncompliant {{(SecretKey) HMAC}} + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the SHA-1 (160 hash bits) hash function and the given secret key. + * + * @param key the secret key + * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC + * @since 20.0 + */ + public static Key hmacSha1(Key key) { + return key; + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the SHA-1 (160 hash bits) hash function and a SecretKeySpec created from the given byte + * array and the SHA-1 algorithm. + * + * @param key the key material of the secret key + * @since 20.0 + */ + public static Key hmacSha1(byte[] key) { + return hmacSha1(new SecretKeySpec(key, "HmacSHA1")); // Noncompliant {{(SecretKey) HMAC}} + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the SHA-256 (256 hash bits) hash function and the given secret key. + * + * @param key the secret key + * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC + * @since 20.0 + */ + public static Key hmacSha256(Key key) { + return key; + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the SHA-256 (256 hash bits) hash function and a SecretKeySpec created from the given byte + * array and the SHA-256 algorithm. + * + * @param key the key material of the secret key + * @since 20.0 + */ + public static Key hmacSha256(byte[] key) { + return hmacSha256(new SecretKeySpec(key, "HmacSHA256")); // Noncompliant {{(SecretKey) HMAC}} + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the SHA-512 (512 hash bits) hash function and the given secret key. + * + * @param key the secret key + * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC + * @since 20.0 + */ + public static Key hmacSha512(Key key) { + return key; + } + + /** + * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using + * the SHA-512 (512 hash bits) hash function and a SecretKeySpec created from the given byte + * array and the SHA-512 algorithm. + * + * @param key the key material of the secret key + * @since 20.0 + */ + public static Key hmacSha512(byte[] key) { + return hmacSha512(new SecretKeySpec(key, "HmacSHA512")); // Noncompliant {{(SecretKey) HMAC}} + } +} diff --git a/java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java b/java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java new file mode 100644 index 000000000..eccd55ecd --- /dev/null +++ b/java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java @@ -0,0 +1,155 @@ +/* + * Sonar Cryptography Plugin + * Copyright (C) 2024 PQCA + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.ibm.plugin.rules.issues; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.fail; + +import com.ibm.engine.detection.DetectionStore; +import com.ibm.engine.model.Algorithm; +import com.ibm.engine.model.IValue; +import com.ibm.engine.model.context.SecretKeyContext; +import com.ibm.mapper.model.IAsset; +import com.ibm.mapper.model.INode; +import com.ibm.plugin.TestBase; +import java.util.List; +import javax.annotation.Nonnull; +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; +import org.sonar.plugins.java.api.JavaCheck; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.Tree; + +/** + * End-to-end location proof for issue #339 using the exact + * pkg:maven/com.google.guava/guava@33.0.0-jre Hashing.java pattern. + * + *

The Guava pattern nests {@code new SecretKeySpec(...)} directly as an argument: + * + * 

+ *   return hmacMd5(new SecretKeySpec(checkNotNull(key), "HmacMD5"));
+ *
+ * + *

The three-layer chain this test proves: + * + *

    + *
  1. Source line: the {@code new SecretKeySpec(...)} call in GuavaHashingTestFile.java + * (lines 65 / 89 / 113 / 137), mirroring Guava lines 306 / 330 / 354 / 378. + *
  2. DetectionLocation.lineNumber(): asserted via + * {@code ((IAsset) nodes.get(0)).getDetectionContext().lineNumber()}. + *
  3. CBOM occurrence line: {@code CBOMOutputFile.java:363} sets + * {@code occurrence.setLine(detectionLocation.lineNumber())} — the same value, verbatim. + *
+ * + *

If all three columns match (source == DetectionLocation == CBOM), the fix is proven. + */ +// https://github.com/cbomkit/sonar-cryptography/issues/339 +class GuavaHashingTest extends TestBase { + + /** + * Verifies that the nested-SecretKeySpec pattern (Guava Hashing.java style) is detected at the + * correct call-site lines, not at adjacent javadoc comments. + * + *

Evidence table (fixture line → Guava source line): + * + * 

+     * Fixture  Guava   Algorithm   DetectionLocation  CBOM occurrence
+     * line 65  line 306  HmacMD5     65               65
+     * line 89  line 330  HmacSHA1    89               89
+     * line 113 line 354  HmacSHA256  113              113
+     * line 137 line 378  HmacSHA512  137              137
+     *
+ */ + @Test + void nestedSecretKeySpecIsDetectedAtCallSiteNotJavadoc() { + CheckVerifier.newVerifier() + .onFile("src/test/files/rules/issues/GuavaHashingTestFile.java") + .withChecks(this) + .verifyIssues(); + } + + @Override + public void asserts( + int findingId, + @Nonnull DetectionStore detectionStore, + @Nonnull List nodes) { + switch (findingId) { + case 0 -> { + // hmacMd5(new SecretKeySpec(key, "HmacMD5")) — fixture line 65, Guava line 306 + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(SecretKeyContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacMD5"); + // Layer 2 → 3: DetectionLocation.lineNumber() == CBOM occurrence.line (same value) + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .as("fixture line 65 == Guava line 306 (HmacMD5 SecretKeySpec)") + .isEqualTo(65); + } + case 1 -> { + // hmacSha1(new SecretKeySpec(key, "HmacSHA1")) — fixture line 89, Guava line 330 + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(SecretKeyContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacSHA1"); + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .as("fixture line 89 == Guava line 330 (HmacSHA1 SecretKeySpec)") + .isEqualTo(89); + } + case 2 -> { + // hmacSha256(new SecretKeySpec(key, "HmacSHA256")) — fixture line 113, Guava line 354 + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(SecretKeyContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacSHA256"); + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .as("fixture line 113 == Guava line 354 (HmacSHA256 SecretKeySpec)") + .isEqualTo(113); + } + case 3 -> { + // hmacSha512(new SecretKeySpec(key, "HmacSHA512")) — fixture line 137, Guava line 378 + assertThat(detectionStore.getDetectionValueContext()) + .isInstanceOf(SecretKeyContext.class); + assertThat(detectionStore.getDetectionValues()).hasSize(1); + IValue value = detectionStore.getDetectionValues().get(0); + assertThat(value).isInstanceOf(Algorithm.class); + assertThat(value.asString()).isEqualTo("HmacSHA512"); + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .as("fixture line 137 == Guava line 378 (HmacSHA512 SecretKeySpec)") + .isEqualTo(137); + } + default -> fail("Unexpected findingId: " + findingId); + } + } +} diff --git a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java index 5f535be47..7800a1876 100644 --- a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java +++ b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java @@ -27,6 +27,7 @@ import com.ibm.engine.model.IValue; import com.ibm.engine.model.context.MacContext; import com.ibm.engine.model.context.SecretKeyContext; +import com.ibm.mapper.model.IAsset; import com.ibm.mapper.model.INode; import com.ibm.plugin.TestBase; import java.util.List; @@ -39,44 +40,32 @@ import org.sonar.plugins.java.api.tree.Tree; /** - * Regression test for issue #339: Detection location off - Findings reported - * below the actual + * Regression test for issue #339: Detection location off - Findings reported below the actual * place. * - *

- * When scanning code that has methods separated by multi-line javadoc comments - * (like Guava's - * Hashing.java), all findings were reported at the closing {@code * /} of the - * next + *

When scanning code that has methods separated by multi-line javadoc comments (like Guava's + * Hashing.java), all findings were reported at the closing {@code * /} of the next * method's javadoc comment instead of at the actual detection site. * - *

- * This test verifies that: + *

This test verifies that: * *

    - *
  1. Findings for {@code new SecretKeySpec(...)} are reported on the line of - * the constructor - * call, not on the javadoc comment of the following method. - *
  2. Findings for {@code Mac.getInstance(...)} are reported on the line of the - * method - * invocation, not on the javadoc comment of the following method. + *
  3. Findings for {@code new SecretKeySpec(...)} are reported on the line of the constructor + * call, not on the javadoc comment of the following method. + *
  4. Findings for {@code Mac.getInstance(...)} are reported on the line of the method + * invocation, not on the javadoc comment of the following method. *
* - *

- * The test fixture {@code PreciseIssueLocationTestFile.java} reproduces the - * Guava Hashing.java - * pattern exactly: each method is separated from the next by a multi-line - * {@code /** ... * /} + *

The test fixture {@code PreciseIssueLocationTestFile.java} reproduces the Guava Hashing.java + * pattern exactly: each method is separated from the next by a multi-line {@code /** ... * /} * javadoc block. */ // https://github.com/cbomkit/sonar-cryptography/issues/339 class PreciseIssueLocationTest extends TestBase { /** - * Verifies that issues are reported at the correct lines (i.e., at the - * {@code //Noncompliant} - * markers in the test fixture, which are placed on the actual call-site lines, - * NOT on the + * Verifies that issues are reported at the correct lines (i.e., at the {@code //Noncompliant} + * markers in the test fixture, which are placed on the actual call-site lines, NOT on the * closing {@code * /} of adjacent javadoc comments). */ @Test @@ -101,6 +90,11 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacMD5"); + // Regression for #339: occurrence must point to the call site (line 23), not javadoc + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .isEqualTo(23); } case 1 -> { // Mac.getInstance("HmacMD5") in hmacMd5(Key key) @@ -110,6 +104,11 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacMD5"); + // Regression for #339: occurrence must point to the call site (line 35), not javadoc + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .isEqualTo(35); } case 2 -> { // new SecretKeySpec(key, "HmacSHA256") in hmacSha256(byte[] key) @@ -119,6 +118,11 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacSHA256"); + // Regression for #339: occurrence must point to the call site (line 53), not javadoc + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .isEqualTo(53); } case 3 -> { // Mac.getInstance("HmacSHA256") in hmacSha256(Key key) @@ -128,9 +132,13 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacSHA256"); + // Regression for #339: occurrence must point to the call site (line 67), not javadoc + assertThat(nodes).isNotEmpty(); + assertThat(nodes.get(0)).isInstanceOf(IAsset.class); + assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) + .isEqualTo(67); } - default -> fail( - "Unexpected findingId: " + findingId); + default -> fail("Unexpected findingId: " + findingId); } } } From b93fc466e0673951b1df5b5e113e585212dd8ee1 Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Mon, 1 Jun 2026 14:11:43 +0530 Subject: [PATCH 4/4] test(java): focus regression on occurrence locations Signed-off-by: Sachin Kumar --- .../rules/issues/GuavaHashingTestFile.java | 139 ---------------- .../plugin/rules/issues/GuavaHashingTest.java | 155 ------------------ .../issues/PreciseIssueLocationTest.java | 12 +- 3 files changed, 8 insertions(+), 298 deletions(-) delete mode 100644 java/src/test/files/rules/issues/GuavaHashingTestFile.java delete mode 100644 java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java diff --git a/java/src/test/files/rules/issues/GuavaHashingTestFile.java b/java/src/test/files/rules/issues/GuavaHashingTestFile.java deleted file mode 100644 index 8d4204589..000000000 --- a/java/src/test/files/rules/issues/GuavaHashingTestFile.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Sonar Cryptography Plugin - * Copyright (C) 2024 PQCA - * - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to you under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import java.security.Key; -import javax.crypto.spec.SecretKeySpec; - -/** - * Reproduction of the pkg:maven/com.google.guava/guava@33.0.0-jre Hashing.java pattern. - * - *

In the real Hashing.java (lines 285-379), each hmacXxx(byte[] key) method passes a nested - * {@code new SecretKeySpec(...)} directly as an argument to the overloaded hmacXxx(Key key) - * variant. Each pair is separated by a multi-line javadoc comment. - * - *

Actual source lines in guava-33.0.0-jre Hashing.java: - * - *

    - *
  • line 306: {@code return hmacMd5(new SecretKeySpec(checkNotNull(key), "HmacMD5"));} - *
  • line 330: {@code return hmacSha1(new SecretKeySpec(checkNotNull(key), "HmacSHA1"));} - *
  • line 354: {@code return hmacSha256(new SecretKeySpec(checkNotNull(key), "HmacSHA256"));} - *
  • line 378: {@code return hmacSha512(new SecretKeySpec(checkNotNull(key), "HmacSHA512"));} - *
- * - *

This fixture uses only JDK types but preserves the exact structural pattern: nested {@code new - * SecretKeySpec(...)} as a direct argument, methods separated by multi-line javadoc. - */ -public class GuavaHashingTestFile { - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the MD5 (128 hash bits) hash function and the given secret key. - * - * @param key the secret key - * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC - * @since 20.0 - */ - public static Key hmacMd5(Key key) { - return key; - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the MD5 (128 hash bits) hash function and a SecretKeySpec created from the given byte array - * and the MD5 algorithm. - * - * @param key the key material of the secret key - * @since 20.0 - */ - public static Key hmacMd5(byte[] key) { - return hmacMd5(new SecretKeySpec(key, "HmacMD5")); // Noncompliant {{(SecretKey) HMAC}} - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the SHA-1 (160 hash bits) hash function and the given secret key. - * - * @param key the secret key - * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC - * @since 20.0 - */ - public static Key hmacSha1(Key key) { - return key; - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the SHA-1 (160 hash bits) hash function and a SecretKeySpec created from the given byte - * array and the SHA-1 algorithm. - * - * @param key the key material of the secret key - * @since 20.0 - */ - public static Key hmacSha1(byte[] key) { - return hmacSha1(new SecretKeySpec(key, "HmacSHA1")); // Noncompliant {{(SecretKey) HMAC}} - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the SHA-256 (256 hash bits) hash function and the given secret key. - * - * @param key the secret key - * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC - * @since 20.0 - */ - public static Key hmacSha256(Key key) { - return key; - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the SHA-256 (256 hash bits) hash function and a SecretKeySpec created from the given byte - * array and the SHA-256 algorithm. - * - * @param key the key material of the secret key - * @since 20.0 - */ - public static Key hmacSha256(byte[] key) { - return hmacSha256(new SecretKeySpec(key, "HmacSHA256")); // Noncompliant {{(SecretKey) HMAC}} - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the SHA-512 (512 hash bits) hash function and the given secret key. - * - * @param key the secret key - * @throws IllegalArgumentException if the given key is inappropriate for initializing this MAC - * @since 20.0 - */ - public static Key hmacSha512(Key key) { - return key; - } - - /** - * Returns a hash function implementing the Message Authentication Code (MAC) algorithm, using - * the SHA-512 (512 hash bits) hash function and a SecretKeySpec created from the given byte - * array and the SHA-512 algorithm. - * - * @param key the key material of the secret key - * @since 20.0 - */ - public static Key hmacSha512(byte[] key) { - return hmacSha512(new SecretKeySpec(key, "HmacSHA512")); // Noncompliant {{(SecretKey) HMAC}} - } -} diff --git a/java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java b/java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java deleted file mode 100644 index eccd55ecd..000000000 --- a/java/src/test/java/com/ibm/plugin/rules/issues/GuavaHashingTest.java +++ /dev/null @@ -1,155 +0,0 @@ -/* - * Sonar Cryptography Plugin - * Copyright (C) 2024 PQCA - * - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to you under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.ibm.plugin.rules.issues; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.fail; - -import com.ibm.engine.detection.DetectionStore; -import com.ibm.engine.model.Algorithm; -import com.ibm.engine.model.IValue; -import com.ibm.engine.model.context.SecretKeyContext; -import com.ibm.mapper.model.IAsset; -import com.ibm.mapper.model.INode; -import com.ibm.plugin.TestBase; -import java.util.List; -import javax.annotation.Nonnull; -import org.junit.jupiter.api.Test; -import org.sonar.java.checks.verifier.CheckVerifier; -import org.sonar.plugins.java.api.JavaCheck; -import org.sonar.plugins.java.api.JavaFileScannerContext; -import org.sonar.plugins.java.api.semantic.Symbol; -import org.sonar.plugins.java.api.tree.Tree; - -/** - * End-to-end location proof for issue #339 using the exact - * pkg:maven/com.google.guava/guava@33.0.0-jre Hashing.java pattern. - * - *

The Guava pattern nests {@code new SecretKeySpec(...)} directly as an argument: - * - * 

- *   return hmacMd5(new SecretKeySpec(checkNotNull(key), "HmacMD5"));
- *
- * - *

The three-layer chain this test proves: - * - *

    - *
  1. Source line: the {@code new SecretKeySpec(...)} call in GuavaHashingTestFile.java - * (lines 65 / 89 / 113 / 137), mirroring Guava lines 306 / 330 / 354 / 378. - *
  2. DetectionLocation.lineNumber(): asserted via - * {@code ((IAsset) nodes.get(0)).getDetectionContext().lineNumber()}. - *
  3. CBOM occurrence line: {@code CBOMOutputFile.java:363} sets - * {@code occurrence.setLine(detectionLocation.lineNumber())} — the same value, verbatim. - *
- * - *

If all three columns match (source == DetectionLocation == CBOM), the fix is proven. - */ -// https://github.com/cbomkit/sonar-cryptography/issues/339 -class GuavaHashingTest extends TestBase { - - /** - * Verifies that the nested-SecretKeySpec pattern (Guava Hashing.java style) is detected at the - * correct call-site lines, not at adjacent javadoc comments. - * - *

Evidence table (fixture line → Guava source line): - * - * 

-     * Fixture  Guava   Algorithm   DetectionLocation  CBOM occurrence
-     * line 65  line 306  HmacMD5     65               65
-     * line 89  line 330  HmacSHA1    89               89
-     * line 113 line 354  HmacSHA256  113              113
-     * line 137 line 378  HmacSHA512  137              137
-     *
- */ - @Test - void nestedSecretKeySpecIsDetectedAtCallSiteNotJavadoc() { - CheckVerifier.newVerifier() - .onFile("src/test/files/rules/issues/GuavaHashingTestFile.java") - .withChecks(this) - .verifyIssues(); - } - - @Override - public void asserts( - int findingId, - @Nonnull DetectionStore detectionStore, - @Nonnull List nodes) { - switch (findingId) { - case 0 -> { - // hmacMd5(new SecretKeySpec(key, "HmacMD5")) — fixture line 65, Guava line 306 - assertThat(detectionStore.getDetectionValueContext()) - .isInstanceOf(SecretKeyContext.class); - assertThat(detectionStore.getDetectionValues()).hasSize(1); - IValue value = detectionStore.getDetectionValues().get(0); - assertThat(value).isInstanceOf(Algorithm.class); - assertThat(value.asString()).isEqualTo("HmacMD5"); - // Layer 2 → 3: DetectionLocation.lineNumber() == CBOM occurrence.line (same value) - assertThat(nodes).isNotEmpty(); - assertThat(nodes.get(0)).isInstanceOf(IAsset.class); - assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) - .as("fixture line 65 == Guava line 306 (HmacMD5 SecretKeySpec)") - .isEqualTo(65); - } - case 1 -> { - // hmacSha1(new SecretKeySpec(key, "HmacSHA1")) — fixture line 89, Guava line 330 - assertThat(detectionStore.getDetectionValueContext()) - .isInstanceOf(SecretKeyContext.class); - assertThat(detectionStore.getDetectionValues()).hasSize(1); - IValue value = detectionStore.getDetectionValues().get(0); - assertThat(value).isInstanceOf(Algorithm.class); - assertThat(value.asString()).isEqualTo("HmacSHA1"); - assertThat(nodes).isNotEmpty(); - assertThat(nodes.get(0)).isInstanceOf(IAsset.class); - assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) - .as("fixture line 89 == Guava line 330 (HmacSHA1 SecretKeySpec)") - .isEqualTo(89); - } - case 2 -> { - // hmacSha256(new SecretKeySpec(key, "HmacSHA256")) — fixture line 113, Guava line 354 - assertThat(detectionStore.getDetectionValueContext()) - .isInstanceOf(SecretKeyContext.class); - assertThat(detectionStore.getDetectionValues()).hasSize(1); - IValue value = detectionStore.getDetectionValues().get(0); - assertThat(value).isInstanceOf(Algorithm.class); - assertThat(value.asString()).isEqualTo("HmacSHA256"); - assertThat(nodes).isNotEmpty(); - assertThat(nodes.get(0)).isInstanceOf(IAsset.class); - assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) - .as("fixture line 113 == Guava line 354 (HmacSHA256 SecretKeySpec)") - .isEqualTo(113); - } - case 3 -> { - // hmacSha512(new SecretKeySpec(key, "HmacSHA512")) — fixture line 137, Guava line 378 - assertThat(detectionStore.getDetectionValueContext()) - .isInstanceOf(SecretKeyContext.class); - assertThat(detectionStore.getDetectionValues()).hasSize(1); - IValue value = detectionStore.getDetectionValues().get(0); - assertThat(value).isInstanceOf(Algorithm.class); - assertThat(value.asString()).isEqualTo("HmacSHA512"); - assertThat(nodes).isNotEmpty(); - assertThat(nodes.get(0)).isInstanceOf(IAsset.class); - assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) - .as("fixture line 137 == Guava line 378 (HmacSHA512 SecretKeySpec)") - .isEqualTo(137); - } - default -> fail("Unexpected findingId: " + findingId); - } - } -} diff --git a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java index 7800a1876..1b1d47876 100644 --- a/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java +++ b/java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java @@ -90,7 +90,8 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacMD5"); - // Regression for #339: occurrence must point to the call site (line 23), not javadoc + // Regression for #339: occurrence must point to the call site (line 23), not + // javadoc assertThat(nodes).isNotEmpty(); assertThat(nodes.get(0)).isInstanceOf(IAsset.class); assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) @@ -104,7 +105,8 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacMD5"); - // Regression for #339: occurrence must point to the call site (line 35), not javadoc + // Regression for #339: occurrence must point to the call site (line 35), not + // javadoc assertThat(nodes).isNotEmpty(); assertThat(nodes.get(0)).isInstanceOf(IAsset.class); assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) @@ -118,7 +120,8 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacSHA256"); - // Regression for #339: occurrence must point to the call site (line 53), not javadoc + // Regression for #339: occurrence must point to the call site (line 53), not + // javadoc assertThat(nodes).isNotEmpty(); assertThat(nodes.get(0)).isInstanceOf(IAsset.class); assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber()) @@ -132,7 +135,8 @@ public void asserts( IValue value = detectionStore.getDetectionValues().get(0); assertThat(value).isInstanceOf(Algorithm.class); assertThat(value.asString()).isEqualTo("HmacSHA256"); - // Regression for #339: occurrence must point to the call site (line 67), not javadoc + // Regression for #339: occurrence must point to the call site (line 67), not + // javadoc assertThat(nodes).isNotEmpty(); assertThat(nodes.get(0)).isInstanceOf(IAsset.class); assertThat(((IAsset) nodes.get(0)).getDetectionContext().lineNumber())