Skip to content

Commit 57b5e5f

Browse files
committed
fix(sonarqube): mdDesc fallback
1 parent 5178368 commit 57b5e5f

3 files changed

Lines changed: 102 additions & 6 deletions

File tree

dojo/tools/api_sonarqube/importer.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import textwrap
44

55
import html2text
6+
import markdown
67
from django.conf import settings
78
from django.core.exceptions import ValidationError
89
from lxml import etree
@@ -153,13 +154,13 @@ def import_issues(self, test):
153154
except KeyError:
154155
sonarqube_permalink = "No permalink \n"
155156

156-
# custom (user defined) SQ rules may not have 'htmlDesc'
157-
if "htmlDesc" in rule:
157+
rule_details = self.get_rule_details(rule)
158+
if rule_details:
158159
description = self.clean_rule_description_html(
159-
rule["htmlDesc"],
160+
rule_details,
160161
)
161-
cwe = self.clean_cwe(rule["htmlDesc"])
162-
references = sonarqube_permalink + self.get_references(rule["htmlDesc"])
162+
cwe = self.clean_cwe(rule_details)
163+
references = sonarqube_permalink + self.get_references(rule_details)
163164
else:
164165
description = ""
165166
cwe = None
@@ -338,8 +339,10 @@ def import_hotspots(self, test):
338339

339340
@staticmethod
340341
def clean_rule_description_html(raw_html):
342+
if not raw_html:
343+
return ""
341344
search = re.search(
342-
r"^(.*?)(?:(<h2>See</h2>)|(<b>References</b>))",
345+
r"^(.*?)(?:(<h2>See</h2>)|(<h2>References</h2>)|(<b>References</b>))",
343346
raw_html,
344347
re.DOTALL,
345348
)
@@ -356,6 +359,21 @@ def clean_cwe(raw_html):
356359
return int(search.group(1))
357360
return None
358361

362+
@staticmethod
363+
def get_rule_details(rule):
364+
if html_desc := rule.get("htmlDesc"):
365+
return html_desc
366+
if not (md_desc := rule.get("mdDesc")):
367+
return ""
368+
if SonarQubeApiImporter.is_html_description(md_desc):
369+
return md_desc
370+
# SonarQube 2025.x can return markdown-only rule descriptions.
371+
return markdown.markdown(md_desc, extensions=["extra"])
372+
373+
@staticmethod
374+
def is_html_description(description):
375+
return bool(re.search(r"<[a-zA-Z][^>]*>", description))
376+
359377
@staticmethod
360378
def convert_sonar_severity(sonar_severity):
361379
sev = sonar_severity.lower()
@@ -382,6 +400,8 @@ def convert_scanner_confidence(sonar_scanner_confidence):
382400

383401
@staticmethod
384402
def get_references(vuln_details):
403+
if not vuln_details:
404+
return ""
385405
parser = etree.HTMLParser()
386406
details = etree.fromstring(vuln_details, parser)
387407

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"key": "typescript:S1854",
3+
"repo": "typescript",
4+
"name": "Dead stores should be removed",
5+
"createdAt": "2018-01-17T10:11:21-0500",
6+
"mdDesc": "A dead store happens when a local variable is assigned a value that is not read by any subsequent instruction. Calculating or retrieving a value only to then overwrite it or throw it away, could indicate a serious error in the code. Even if it's not an error, it is at best a waste of resources. Therefore all calculated values should be used.\n\n## Noncompliant Code Example\n\ni = a + b; // Noncompliant; calculation result not used before value is overwritten\ni = compute();\n\n## Compliant Solution\n\ni = a + b;\ni += compute();\n\n## Exceptions\n\nThis rule ignores initializations to -1, 0, 1, `null`, `true`, `false`, `\"\"`, `[]` and `{}`.\n\n## See\n\n- [MITRE, CWE-563](http://cwe.mitre.org/data/definitions/563.html) - Assignment to Variable without Use ('Unused Variable')\n- [CERT, MSC13-C.](https://www.securecoding.cert.org/confluence/x/QYA5) - Detect and remove unused values\n- [CERT, MSC56-J.](https://www.securecoding.cert.org/confluence/x/uQCSBg) - Detect and remove superfluous code and values\n",
7+
"severity": "MAJOR",
8+
"status": "READY",
9+
"isTemplate": false,
10+
"tags": [],
11+
"sysTags": [
12+
"cert",
13+
"cwe",
14+
"unused"
15+
],
16+
"lang": "ts",
17+
"langName": "TypeScript",
18+
"params": [],
19+
"defaultDebtRemFnType": "CONSTANT_ISSUE",
20+
"defaultDebtRemFnOffset": "15min",
21+
"debtOverloaded": false,
22+
"debtRemFnType": "CONSTANT_ISSUE",
23+
"debtRemFnOffset": "15min",
24+
"defaultRemFnType": "CONSTANT_ISSUE",
25+
"defaultRemFnBaseEffort": "15min",
26+
"remFnType": "CONSTANT_ISSUE",
27+
"remFnBaseEffort": "15min",
28+
"remFnOverloaded": false,
29+
"scope": "MAIN",
30+
"isExternal": false,
31+
"type": "CODE_SMELL"
32+
}

unittests/tools/test_api_sonarqube_importer.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ def dummy_rule_wo_html_desc(self, *args, **kwargs):
2828
return json.load(json_file)
2929

3030

31+
def dummy_rule_md_desc_only(self, *args, **kwargs):
32+
with (get_unit_tests_scans_path("api_sonarqube") / "rule_md_desc_only.json").open(encoding="utf-8") as json_file:
33+
return json.load(json_file)
34+
35+
3136
def dummy_no_hotspot(self, *args, **kwargs):
3237
with (get_unit_tests_scans_path("api_sonarqube") / "hotspots" / "no_vuln.json").open(encoding="utf-8") as json_file:
3338
return json.load(json_file)
@@ -293,6 +298,45 @@ def test_parser(self):
293298
self.assertEqual("internal.dummy.project:src/main/javascript/TranslateDirective.ts", finding.file_path)
294299

295300

301+
class TestSonarqubeImporterMarkdownRuleDescription(DojoTestCase):
302+
fixtures = [
303+
"unit_sonarqube_toolType.json",
304+
"unit_sonarqube_toolConfig1.json",
305+
"unit_sonarqube_toolConfig2.json",
306+
"unit_sonarqube_product.json",
307+
"unit_sonarqube_sqcNoKey.json",
308+
"unit_sonarqube_sqcWithKey.json",
309+
]
310+
311+
def setUp(self):
312+
product = Product.objects.get(name="product")
313+
engagement = Engagement(product=product)
314+
self.test = Test(
315+
engagement=engagement,
316+
api_scan_configuration=Product_API_Scan_Configuration.objects.all().last(),
317+
)
318+
319+
@mock.patch("dojo.tools.api_sonarqube.api_client.SonarQubeAPI.get_project", dummy_product)
320+
@mock.patch("dojo.tools.api_sonarqube.api_client.SonarQubeAPI.get_rule", dummy_rule_md_desc_only)
321+
@mock.patch("dojo.tools.api_sonarqube.api_client.SonarQubeAPI.find_issues", dummy_issues)
322+
@mock.patch("dojo.tools.api_sonarqube.api_client.SonarQubeAPI.get_hotspot_rule", dummy_hotspot_rule)
323+
@mock.patch("dojo.tools.api_sonarqube.api_client.SonarQubeAPI.find_hotspots", empty_list)
324+
def test_parser(self):
325+
parser = SonarQubeApiImporter()
326+
findings = parser.get_findings(None, self.test)
327+
self.assertEqual(2, len(findings))
328+
finding = findings[0]
329+
self.assertEqual(563, finding.cwe)
330+
self.assertIn(
331+
"A dead store happens when a local variable is assigned a value",
332+
finding.description,
333+
)
334+
self.assertIn(
335+
"[MITRE, CWE-563](http://cwe.mitre.org/data/definitions/563.html)",
336+
finding.references,
337+
)
338+
339+
296340
class TestSonarqubeImporterTwoIssuesNoHotspots(DojoTestCase):
297341
# Testing case no 9. https://github.com/DefectDojo/django-DefectDojo/pull/4107
298342
fixtures = [

0 commit comments

Comments
 (0)