Skip to content

Allow to dismiss vulnerabilities in security reports

Olivier Gonzalez requested to merge 5151_allow_to_dismiss_vulnerabilities into master

What does this MR do?

Add the new VulnerabiltyFeedback ressource with following actions:

  • index: no restrictons
  • create: restricted to project member with at least developer role
  • destroy: restricted to project member with at least developer role

Frontend checklist:

  • Create a reusable modal that works for all the issues
  • Save modal data in the store
  • Transform each name into a button that opens the modal
  • Handle dismiss request
  • Handle revert dimiss request
  • Handle new issue request
  • Handle errors
  • Fetch, for each report, the list of the dimissed issues
  • Compare which ones are dismissed and show them as dismissed
  • Toggle dismiss / revert dimissed button every time an issue is dimissed
  • Render information in the modal
  • Add tests
  • Mark vulnerability as dismissed/undismissed in the list without page reload
  • Link to file is not working in modal (need to prepend blob path + append line)
  • Send MR’s current pipeline_id with feedback creation request

Backend checklist:

  • allow any role to list feedbacks
  • allow developer role to create/destroy feedback
  • implement category and feedback_type filters on index
  • add missing tests
  • create documentation page
  • improve issue description text with relevant data
  • expose necessary paths to FE (endpoints base, created issue's url)

Best Effort:

  • [FE] Put dismissed vulnerabilities at the end of the list
  • [FE/BE] Display pipeline and author of dismissal

Implementation details:

The added resource is a vulnerability_feedback and the available endpoints are:

Action Url Method Params
Index :project_id/vulnerability_feedbacks GET QueryString: project_id, category (filter), feedback_type (filter)
Create :project_id/vulnerability_feedbacks POST QueryString: project_id. Body: vulnerability_feedback wrapping the following properties: category, feedback_type, project_fingerprint, pipeline_id, vulnerability_data
Destroy :project_id/vulnerability_feedbacks/:id DELETE QueryString: project_id, id
  • category: can be one of [sast, dependency_scanning, container_scanning, dast]
  • feedback_type: can be one of [dismissal, issue]
  • project_fingerprint: a sha1 hex value that is built on different properties depending on the report categoy.
  • pipeline_id: id of the pipeline the vulnerability comes from.
  • vulnerability_data': the vulnerability object as given by the security report. (These values are not yet stored in DB, only used for creating the issue)

Here is a sample JSON output for the index:

[[
  {
    "id": 66,
    "project_id": 19,
    "author_id": 1,
    "issue_id": null,
    "pipeline_id": 162,
    "category": "sast",
    "feedback_type": "dismissal",
    "branch": "feature-branch",
    "project_fingerprint": "c0e74d1435b60fa3cae63bd15deddc3ce40de5be"
  },
  {
    "id": 69,
    "project_id": 19,
    "author_id": 1,
    "issue_id": 260,
    "pipeline_id": 162,
    "issue_url": "http://192.168.0.10:3000/root/security-reports/issues/34",
    "category": "sast",
    "feedback_type": "issue",
    "branch": "feature-branch",
    "project_fingerprint": "5a1f9e4ee29d64499c963214bf67ef79b463f8d1"
  }
]

Are there points in the code the reviewer needs to double check?

As this is the first feature made by Security-Products team on GitLab backend code please review carefully.

  • the Dismiss feature is EE only so the code should land in EE scope as much as possible.
  • DB migration added (new table). Please review column types, constraints, indexes and foreign keys.

Why was this MR needed?

User should be able to dismiss vulnerabilities in security reports when it is a false positive or it has been fixed using a workaround, etc. User should also be able to create an issue filled with vulnerability details to investigate and try to resolve it.

Screenshots (if relevant)

Modal Created issue
modal created issue

Does this MR meet the acceptance criteria?

  • Changelog entry added, if necessary
  • Documentation created/updated
  • [ ] API support added
  • Tests added for this feature/bug
  • Review
    • Has been reviewed by UX
    • Has been reviewed by Frontend
    • Has been reviewed by Backend
    • Has been reviewed by Database
  • EE specific content should be in the top level /ee folder
  • Conform by the merge request performance guides
  • Conform by the style guides
  • Squashed related commits together
  • Internationalization required/considered
  • If paid feature, have we considered GitLab.com plan and how it works for groups and is there a design for promoting it to users who aren't on the correct plan
  • End-to-end tests pass (package-qa manual pipeline job)

What are the relevant issue numbers?

Refs #5151 (closed)

Edited by Olivier Gonzalez

Merge request reports