fix(scripts): remove dead sonar-report dep and re-enable container cleanup#446
Open
TheAuditorTool wants to merge 1 commit intoOWASP-Benchmark:masterfrom
Open
Conversation
…eanup Closes OWASP-Benchmark#187. The jq ARG_MAX overflow was fixed in 32933c4 by replacing shell-based result aggregation with SonarReport.java. This commit removes leftover artifacts: the sonar-report tool check that blocks script execution despite no longer being used, and uncomments docker stop so the SonarQube container is properly cleaned up after a run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #187 reported
/usr/bin/jq: Argument list too longwhen runningscripts/runSonarQube.sh. This was caused by the original script accumulating thousands of SonarQube vulnerability results in shell variables and passing them as command-line arguments tojq, which exceeded Linux'sARG_MAXlimit (~2MB).The core fix was already applied in commit
32933c4e6(Feb 2025) by @darkspirit510, which replaced the shell-basedjqresult aggregation with a native Java class (SonarReport.java) that paginates the SonarQube API and writes results directly to disk. This PR cleans up leftover artifacts from the intermediate fix that were missed.Problem Chain (full history)
459108114jqaccumulates all results in shell variables. Breaks at ~2MB of JSON.b697dc9534fb517b57jqprocessing with externalsonar-reportGo tool. Addedsonar-reportdependency check.32933c4e6sonar-reportwith nativeSonarReport.java. But thesonar-reportdependency check was left behind.sonar-reportcheck and re-enables container cleanup.Changes
1. Removed dead
sonar-reportdependency checkThe script still had:
This
exit 1blocked anyone from running the script unless they hadsonar-reportinstalled -- a tool that is no longer used since commit32933c4e6replaced it withmvn exec:java -Dexec.mainClass="org.owasp.benchmark.report.sonarqube.SonarReport".2. Re-enabled container cleanup
The
docker stopwas commented out in32933c4e6, leaving a running SonarQube container after every script run.What
jqis still used for (and why it's fine)The script retains 4
jqcalls, all operating on small single-object API responses (not result aggregation):docker inspectNone of these can approach
ARG_MAX. TherequireCommand jqcheck is retained because these calls still need it.Deferred findings (out of scope for #187)
During the audit of
SonarReport.java, three issues were identified that are unrelated to thejqoverflow but worth tracking separately:No HTTP error checking in
apiCall()(SonarReport.java:106-113):HttpURLConnectionresponses are read fromgetInputStream()without checking the status code. Non-2xx responses (e.g., 400, 401, 500) will throw anIOExceptionwith no useful error context, or in some cases silently return an error body that breaks Jackson deserialization.SonarQube 10K result hard cap (SonarReport.java:84-100): The
issues/searchAPI enforces a hard limit of 10,000 results (p * ps <= 10000). AtPAGE_SIZE = 500, page 21 would request offset 10,500 and receive an HTTP 400. For Enterprise editions reporting 18K+ vulnerabilities (as noted by @zoobinn in /usr/bin/jq: Argument list too long #187), this would silently truncate results or crash.Off-by-one in page count (SonarReport.java:94):
(total / PAGE_SIZE) + 1uses integer division and unconditionally adds 1. When total is an exact multiple of 500, this fires one extra empty-page request. Harmless but wasteful.Test plan
scripts/runSonarQube.shon a machine with Docker but withoutsonar-reportinstalled -- should no longer exit with "sonar-report is required"sonarqube-benchmarkcontainer is running (docker ps)results/Benchmark_*-sonarqube-v*.jsonis written with bothissuesandhotspotsarrays