Skip to content

Include SAST findings in inline diff view (backend)

Ahmed Hemdan requested to merge include-sast-findings-in-inline-diff-view into master

What does this MR do and why?

This merge request introduces a new controller action sast_mr_diff_reports under EE::Projects::MergeRequestsController controller with the following path:

/merge_requests/:id/sast_mr_diff_reports

The endpoint is behind a feature flag with the same name, sast_mr_diff_reports, to make sure we can roll this out easily and turn the feature on and off, as this is considered a new feature in a high traffic area.

In addition to that, the endpoint reuses some of the abstractions in the merge_requests controller to be consistent with other security reports comparison endpoints (e.g. codequality_mr_diff_reports).

However, the comparison is handled by a new comparison service class Ci::CompareSastReportsService instead of the Ci::CompareSecurityReportsService, as the latter is likely not the best option at the moment for doing such comparisons (see this comment for more information).

Please note: I'm still ironing a few edge cases out, but I thought I'd start having eyes looking at this to gather feedback and ensure we’re on the right path.

Resolves #389867 (closed).

How to set up and validate locally

  • Start by enabling on the feature flag from your rails console:
Feature.enable(:sast_mr_diff_reports)
  • Create a project if you don't have one already, and add a .gitlab-ci.yml file inside the project.
  • Make sure to have include sast template in the .gitlab-ci.yml file
  include:
    template: Jobs/SAST.gitlab-ci.yml
  • Add some code with vulnerabilities to the project (check sample projects for inspiration).
  • Create a new merge request with a piece of code causing a vulnerability.
  • Browse to http://gdk.test:3000/:namespace/:project_slug/-/merge_requests/:id/sast_mr_diff_reports.
    • Make sure to replace :namespace with the namespace name or slug.
    • Also, replace :project_slug with the correct slug of your project.
    • Then, replace :id with the id of the merge request.
  • You should receive a JSON response including new and existing findings for this merge request.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Ahmed Hemdan

Merge request reports