test(java): verify occurrence locations for issue #339#430
Conversation
Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test to ensure findings are reported on the exact call-site lines (not shifted to adjacent Javadoc comment boundaries), addressing issue #339.
Changes:
- Introduces a new JUnit test that verifies reported issue locations using
CheckVerifier. - Adds a dedicated Java test fixture file reproducing the “methods separated by multi-line Javadoc” pattern.
- Validates detected algorithm/context values for multiple findings (HmacMD5, HmacSHA256).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| java/src/test/java/com/ibm/plugin/rules/issues/PreciseIssueLocationTest.java | Adds regression test + assertions for detected contexts/algorithms to validate precise issue locations. |
| java/src/test/files/rules/issues/PreciseIssueLocationTestFile.java | Adds fixture code with Javadoc-separated methods and // Noncompliant markers to reproduce the location-shift bug. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@san-zrl @n1ckl0sk0rtge |
Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>
|
Hi @sachin9058 - This PR only confirms what we already know: We detect the correct crypto calls. Your code does not assert that the detected locations correspond to the real locations of the crypto calls in the code. #339 is not solved yet. Reproducing this is cumbersome for you. You have to clone the latest versions of the main branches of sonar-cryptography, cbomkit-lib and cbomkit, compile everything locally making sure that cbomkit-lib and cbomkit use the local packages as dependencies and run cbonkit on the purl pkg:maven/com.google.guava/guava@33.0.0-jre mentioned in #339. Fell free to explore this further. We are still interested in an explanation and a fix for the problem. Closing this for the moment. |
|
@san-zrl I reproduced issue #339 locally using CBOMkit against: pkg:maven/com.google.guava/guava@33.0.0-jre and validated the generated CBOM output through: /api/v1/cbom/last/10 The resulting occurrences now point to the actual crypto call locations inside: guava/src/com/google/common/hash/Hashing.java Examples from generated CBOM:
instead of pointing to the trailing Javadoc closing ( This confirms the refined token selection logic correctly propagates into final exported CBOM evidence locations. |
|
@san-zrl Thanks for taking another look. |
|
Hi @sachin9058 - Your PR is titled "add regression coverage for detection location". This is not quite was it does. The name of the additional test Moreover, the issue still persists. See my recent comment in #339. |
Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>
|
@san-zrl I've updated the investigation with explicit location assertions. Changes:
The tests now verify the translated occurrence location directly via Verification: mvn -pl java test -Dtest=PreciseIssueLocationTest,GuavaHashingTestResult: The assertions verify that the occurrence locations resolve to the crypto call sites rather than the surrounding Javadoc structure. |
Description
Adds regression coverage for issue #339 by verifying occurrence locations, not only that detections are produced.
The tests reproduce the Guava
Hashing.javapattern that originally triggered the report: HMAC methods separated by multi-line Javadoc blocks.Changes
DetectionContext.lineNumber()inPreciseIssueLocationTestGuavaHashingTestGuavaHashingTestFileVerification
Executed locally:
mvn -pl java test -Dtest=PreciseIssueLocationTest,GuavaHashingTest
Result:
Tests run: 2
Failures: 0
Errors: 0
Skipped: 0
BUILD SUCCESS
The tests assert occurrence line numbers directly via:
((IAsset) node).getDetectionContext().lineNumber()
which is the same location object propagated into the generated CBOM occurrence.