Support multiple sites in DAST reports
What does this MR do?
Up to this point, DAST reports contained a single site
that was represented in the form of an Object
. We now want to support cases where DAST reports can contain multiples sites, where the site
property will be an Array
instead of an Object
. In fact, zaproxy was updated to normalize the site
property into an Array
, even when there's a single site.
In this MR, support for multiple sites has been implemented, while preserving support for the legacy format with site
as an Object
.
Here's a DAST report with 2 sites that can be used for testing this MR: gl-dast-report.json
Some notes about the UX
- The same vulnerability appears 2 times, without any clear distinction between the two occurrences
- This could occur in production and is expected because it's possible for one vulnerability to be reported in two different sites.
- It would be relatively easy to show the site's
@name
in the vulnerabilities list and/or the modal to clear out some of the confusion that comes with having duplicated entries. This would most likely require someone from UX to come up with the best way to make this distinction clear to the users. - When dismissing one of the occurrence then refreshing the page, both occurrences are marked as dismissed because they both have the same identifier (see https://gitlab.com/gitlab-org/gitlab-ee/issues/5678)
- One way of quick fixing this would be to include the
@name
property when generating theproject_fingerprint
in the frontend but this would cause some backward compatibility issues since previously dismissed vulnerabilities would have their hash updated, thus being marked as non-dismissed. So I would say this is out of the question.- I've asked about this issue in the Vulnerability fingerprints overview and Q&A and it looks like we will still have colliding fingerprints once the hashing has been updated and moved to the backend and there doesn't seem to be a consensus on how we should address this going forward. In light of this information, I'll assume that this issue is acceptable for now and I won't do anything to mitigate it unless someone else raises some concerns.
- One way of quick fixing this would be to include the
Does this MR meet the acceptance criteria?
Conformity
Performance and testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Closes frontend part for #11930 (closed)
Edited by Paul Gascou-Vaillancourt