Skip to content

feat(sonarqube): title REST API JSON findings with the issue message - #16334

Open
cvas22 wants to merge 4 commits into
DefectDojo:devfrom
cvas22:fix/sonarqube-json-title-16115
Open

cvas22 wants to merge 4 commits into
DefectDojo:devfrom
cvas22:fix/sonarqube-json-title-16115

Conversation

@cvas22

@cvas22 cvas22 commented Oct 14, 2026 •

Copy link
Copy Markdown

Closes #16115.

Summary

When you import SonarQube REST API JSON (/api/issues/search, /api/hotspots/search, or a zip of those files), each finding is titled <rule>_<key>, for example typescript:S1533_fjoiewfjoweifjoihugu-. SonarQube API Import titles the same issues with SonarQube's message. This PR gives SonarQube Scan detailed the message titles, and leaves SonarQube Scan as it is:

  • In detailed mode, bugs, vulnerabilities, code smells and hotspots take their title from message. A message longer than 511 characters is cut to 507 characters plus ..., the same rule as dojo/tools/api_sonarqube/importer.py.
  • If message is missing, null, empty or only whitespace, the title falls back to <rule>_<key>. rule, key and message are still in the description, and unique_id_from_tool is unchanged.
  • A missing or null message no longer raises TypeError. This crash also happens on dev today.
  • The Soprasteria HTML/JSON paths are not touched.

Why only detailed mode

"SonarQube Scan" hashes on cwe, severity and file_path. The parser leaves cwe at 0 for bugs, code smells and hotspots, and HASHCODE_ALLOWS_NULL_CWE["SonarQube Scan"] is False, so for those findings the hash falls back to the legacy hash, which includes the title. Changing their title there would close and re-create every such finding on the first reimport after upgrading (see the review). So that mode keeps <rule>_<key>, and its hashes don't change.

"SonarQube Scan detailed" deduplicates on unique_id_from_tool (the SonarQube key), so the title isn't part of the match. On reimport, findings imported before the upgrade still match and keep their old title; new findings get the message title.

Changes

  • dojo/tools/sonarqube/sonarqube_restapi_json.py: a new get_title() helper, used for all four finding types, returns the message only in detailed mode. Each message read now defaults to "".

  • unittests/tools/test_sonarqube_parser.py: two new tests, both failing on dev:

    • test_detailed_parse_json_file_issue_16115_title_from_message covers a normal message, truncation, and the empty, whitespace, missing and null fallbacks, for issues and hotspots.
    • test_parse_json_file_issue_16115_keeps_rule_key_title checks that "SonarQube Scan" keeps <rule>_<key> for every type.

    The existing title assertions are unchanged.

  • unittests/scans/sonarqube/issue_16115.json: a small fabricated fixture.

  • docs/content/supported_tools/parsers/file/sonarqube.md: a new "Finding Title" section explaining the behavior of each mode.

No upgrade note: "SonarQube Scan" titles and hashes don't change, and detailed mode matches on unique_id_from_tool.

Test results

  • unittests.tools.test_sonarqube_parser: 24 tests OK in Docker.
  • uvx [email protected] check on the changed Python files: passed.
  • Rebased on the latest dev.
  • I couldn't set the milestone from a fork. The next open milestone is 3.4.200.

@Maffooch Maffooch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review: feat(sonarqube): title REST API JSON findings with the issue message

Verdict: request-changes
Reviewed at: a90b644 Profile: code-change (parser)
Repo rules followed: origin/dev .claude/skills/review-pr/SKILL.md, .claude/skills/defectdojo-parser/SKILL.md (dedup-change section), plus the generic review-pr skill

Attempts to break it

  1. Upgrade reimport, "SonarQube Scan" default dedup. Imported issue_16115.json with the pre-PR title (get_title patched to rule_key), then reimported the same file with the PR code, close_old_findings=True. Result: new=1 closed=1. The BUG finding (id 9, Java:S2259_AZb-Issue-16115-3) was mitigated and re-created as id 10 with the message title. Same run on findings_over_api.json: new=2 closed=3; the BUG and CODE_SMELL findings (ids 12, 13) were closed and re-created (ids 16, 17). Blocker.
    Cause: the parser never sets cwe for BUG, CODE_SMELL or hotspot findings, so cwe stays at the model default 0 (dojo/finding/models.py:121). HASHCODE_ALLOWS_NULL_CWE["SonarQube Scan"] is False (dojo/settings/settings.dist.py:1534), so compute_hash_code falls back to the legacy hash (dojo/finding/models.py:858-862), which hashes title + cwe + line + file_path + description (dojo/finding/models.py:899-900). Changing the title changes hash_code for every BUG, CODE_SMELL and hotspot finding that has a non-empty message.
  2. Control: fresh install, no upgrade. Import then reimport the same file, both with PR code. new=0 closed=0 untouched=3. No break; the problem is only for data imported before the upgrade.
  3. Upgrade reimport, "SonarQube Scan detailed". Same as attempt 1 with the detailed scan type (DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL, settings.dist.py:1723). new=0 closed=0. No break. Old titles are kept on matched findings.
  4. Missing or null message. Parsed a REST API JSON doc with the key absent and with "message": null, for BUG, VULNERABILITY, CODE_SMELL and a hotspot. Both PR and origin/dev raise TypeError ("**message:** " + message at sonarqube_restapi_json.py:47, and "Category: CWE-" in message at line 80). Reproduces on base: follow-up. Note that the new get_title fallback comment ("when the report carries no message") is only reachable for "", never for None.
  5. Whitespace-only message (" "). PR titles the finding " " (visually blank). Base gave r:1_k1. Introduced by the PR but low impact. Nit: use message.strip() in the emptiness check.
  6. Length boundaries. 511-char message gives a 511-char title; 512 chars gives 510 (507 + ...). Both fit Finding.title max_length=511 (dojo/finding/models.py:105) and match dojo/tools/api_sonarqube/importer.py:187. No break.
  7. Hostile input. Message <script>x</script>, RLO mark, Hebrew, emoji: title kept verbatim and saved; output escaping is the template's job, same as SonarQube API Import. Message with \n produces a multi-line title. No break; nit at most.
  8. The tests themselves. The new test test_parse_json_file_issue_16115_title_from_message and the 9 changed assertions all assert the new titles, so they fail on base. No test covers the reimport/dedup behaviour that attempt 1 shows.

Blocking

  • dojo/tools/sonarqube/sonarqube_restapi_json.py:54, :208, :262 (BUG, CODE_SMELL, hotspot branches): the title change alters hash_code for existing findings because these findings have cwe=0 and "SonarQube Scan" falls back to the legacy, title-based hash. On the first reimport after upgrade, every such finding with a message is closed and re-created as a new finding (reproduced, attempt 1). Product-level dedup against older tests breaks the same way.
  • docs/content/supported_tools/parsers/file/sonarqube.md (new "Finding Title" section) and the PR description state that the title is not part of the dedup hash and that only a legacy dedup override is affected. That is wrong for the default config. Per the parser skill's dedup-change rule, this needs: an accurate upgrade note in docs/content/releases/os_upgrading/ for the release it ships in, the PR description corrected, and ideally a design that does not churn existing findings (for example keep the old title on the hash path, set cwe-independent hashing, or make the new title opt-in). manage.py dedupe --hash_code_only does not help here because the stored findings still carry the old title.

Nice to have

  • sonarqube_restapi_json.py:284: treat whitespace-only messages as empty (if not message or not message.strip()).
  • sonarqube_restapi_json.py:282-283: the comment implies a missing message falls back to rule_key, but a missing/null message crashes earlier at line 47 / 80 (pre-existing). Either guard message with or "" at the issue.get("message") sites or fix the comment. Follow-up if left out of this PR.
  • Finding title-casing on save turns "boolean" into "Boolean" inside the message (seen in attempt 1, id 16). Same as SonarQube API Import today; not this PR's to fix.
  • PR has no milestone set (repo checklist). Not set by the reviewer.

Verified hands-on

  • manage.py test unittests.tools.test_sonarqube_parser plus a local upgrade test (not part of the PR), in a compose stack on the PR head: 27 tests OK.
  • Upgrade-simulation reimports for "SonarQube Scan" (two fixtures) and "SonarQube Scan detailed" (attempts 1 to 3).
  • Edge-case parsing on PR head and on origin/dev (attempts 4 to 7).
  • uvx [email protected] check on the two changed Python files: all checks passed. scripts/check_migration_leaves.py: single leaf (no migrations in the PR).

Checked only statically

  • Rendering of message titles in the UI (escaping of markup, RTL, newlines).
  • Hotspot upgrade path: same cwe=0 legacy-hash path as BUG/CODE_SMELL by reading the code; the hands-on run covered BUG and CODE_SMELL only.

Still needs verification

  • Required CI has not run: on the head SHA, Ruff Linter, Unit tests, Migration Graph and the other pull_request workflows are action_required (fork workflows awaiting maintainer approval). They must be approved and pass before merge.

"SonarQube Scan" imports of /api/issues/search and /api/hotspots/search
JSON titled findings as <rule>_<key>, while "SonarQube API Import" uses
the SonarQube message. Use the message here too (truncated like the API
importer), falling back to <rule>_<key> when the message is empty.

The title is not part of the SonarQube Scan dedup hash, so existing
findings still match on reimport and keep their old title.

Closes DefectDojo#16115
SonarQube Scan falls back to the title-based legacy hash for findings
without a CWE, so changing their title closed and re-created existing
findings on reimport. Keep rule_key titles there and use the message
only in SonarQube Scan detailed, which deduplicates on
unique_id_from_tool. Treat missing, null and whitespace-only messages
as empty instead of crashing.
@cvas22
cvas22 force-pushed the fix/sonarqube-json-title-16115 branch from 016a892 to 6692470 Compare October 17, 2026 01:20
@cvas22

cvas22 commented Oct 17, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review. The blocker was right: for bugs, code smells and hotspots without a CWE, the hash falls back to the title-based legacy hash. I went with the opt-in route, in 6692470:

  • "SonarQube Scan" keeps <rule>_<key>, so its titles and hashes are unchanged and existing findings keep matching on reimport. I restored the existing title assertions, and a new test pins <rule>_<key> for all four finding types.
  • "SonarQube Scan detailed" uses the message title. It deduplicates on unique_id_from_tool, which your attempt 3 showed doesn't churn.
  • Missing, null and whitespace-only messages now fall back to <rule>_<key> instead of raising TypeError. The fixture now covers each of these cases, for issues and hotspots.
  • I rewrote the docs and the PR description, and removed the wrong claim that the title isn't in the hash. There's no upgrade note, since the default mode's behavior doesn't change.

The 24 SonarQube parser tests pass, and the two new tests fail on dev. The branch is rebased on the latest dev. I can't set the milestone from a fork; the next open one is 3.4.200.

@dryrunsecurity

Copy link
Copy Markdown

DryRun Security

This pull request contains multiple low-severity findings where user 'cvas22' modified files in sensitive codepaths without being on the allowed authors list. These changes affect various Python and HTML files within the 'dojo' directory but are not blocking.

Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/api_v2/exception_handler.py (drs_2e72c4bd)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/api_v2/exception_handler.py' matches configured sensitive codepath pattern 'dojo/api_v2/*.py' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/base_importer.py (drs_5e2a07d6)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/importers/base_importer.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/default_reimporter.py (drs_367e8d96)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/importers/default_reimporter.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/import_lock.py (drs_9dae1cb9)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/importers/import_lock.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/tasks.py (drs_c78256ae)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/tasks.py' matches configured sensitive codepath pattern 'dojo/tasks.py' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/templates/dojo/filter_js_snippet.html (drs_9b9bb00d)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/templates/dojo/filter_js_snippet.html' matches configured sensitive codepath pattern 'dojo/templates/**/*.html' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/templates/dojo/findings_list_snippet.html (drs_f49776ff)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/templates/dojo/findings_list_snippet.html' matches configured sensitive codepath pattern 'dojo/templates/**/*.html' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/templates/dojo/view_test.html (drs_41690f8e)
Vulnerability Configured Sensitive Codepath Modified by Non-Allowed Author
Description File 'dojo/templates/dojo/view_test.html' matches configured sensitive codepath pattern 'dojo/templates/**/*.html' and was modified by 'cvas22' (commit fa7811f) who is not in the allowed authors list.

Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants