Make sast and secret_detection methods available to CE

The frontend work for the epic: &4388 has been blocked as we do not have the SAST and Secret Detection vulnerabilities comparison endpoints available to CE. In this issue, we will move those endpoints to CE and investigate the side-effect for these changes. This will unblock frontend work. Additionally, we need to move some more methods from EE to CE. Here is an attempt: !43716 (closed). We will try to investigate this draft MR and try to provide a solution.

Where we are at (as of March 5th, 2021)

Merged and Reverted

Draft MR (57 files changed)

Draft MR (87 files changed)

Merged - several changes were made, but underlying classes are still in EE, so feature will not work in CE yet

Merged - behind Feature Flag, cannot be enabled until this Issue is complete

Merged

Known remaining work

Files changed/moved in Saikat's draft MRs Draft: Move security reports comparer to CE and Draft: Move models related to vulnerabilities to CE

Either one or both of these MRs will need to be finished, or a new MR will need to be started based on the work that they've done.

  • app/finders/security/pipeline_vulnerabilities_finder.rb
  • app/models/ci/pipeline.rb
  • app/models/project.rb
  • app/models/vulnerabilities/export.rb
  • app/models/vulnerabilities/external_issue_link.rb
  • app/models/vulnerabilities/feedback.rb
  • app/models/vulnerabilities/finding.rb
  • app/models/vulnerabilities/finding_identifier.rb
  • app/models/vulnerabilities/finding_link.rb
  • app/models/vulnerabilities/finding_pipeline.rb
  • app/models/vulnerabilities/finding_remediation.rb
  • app/models/vulnerabilities/historical_statistic.rb
  • app/models/vulnerabilities/identifier.rb
  • app/models/vulnerabilities/issue_link.rb
  • app/models/vulnerabilities/projects_grade.rb
  • app/models/vulnerabilities/remediation.rb
  • app/models/vulnerabilities/scanner.rb
  • app/models/vulnerabilities/stat_diff.rb
  • app/models/vulnerabilities/statistic.rb
  • app/policies/base_policy.rb
  • app/policies/project_policy.rb
  • app/policies/vulnerabilities/export_policy.rb
  • app/policies/vulnerabilities/feedback_policy.rb
  • app/policies/vulnerabilities/issue_link_policy.rb
  • app/policies/vulnerabilities/scanner_policy.rb
  • app/serializers/vulnerabilities/feedback_entity.rb
  • app/serializers/vulnerabilities/feedback_serializer.rb
  • app/serializers/vulnerabilities/finding_diff_serializer.rb
  • app/serializers/vulnerabilities/finding_entity.rb
  • app/serializers/vulnerabilities/finding_reports_comparer_entity.rb
  • app/serializers/vulnerabilities/finding_serializer.rb
  • app/serializers/vulnerabilities/identifier_entity.rb
  • app/serializers/vulnerabilities/request_entity.rb
  • app/serializers/vulnerabilities/response_entity.rb
  • app/serializers/vulnerabilities/scanner_entity.rb
  • app/services/ci/compare_security_reports_service.rb
  • ee/app/models/ee/ci/pipeline.rb
  • ee/app/models/ee/project.rb
  • ee/app/policies/ee/base_policy.rb
  • ee/app/policies/ee/project_policy.rb
  • ee/changelogs/unreleased/move_security_reports.yml
  • ee/changelogs/unreleased/move_vulnerability_models.yml
  • lib/gitlab/ci/reports/security/aggregated_report.rb
  • lib/gitlab/ci/reports/security/report.rb
  • lib/gitlab/ci/reports/security/reports.rb
  • lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
  • spec/factories/ci/reports/security/aggregated_reports.rb
  • spec/factories/ci/reports/security/reports.rb
  • spec/factories/users.rb
  • spec/factories/vulnerabilities/exports.rb
  • spec/factories/vulnerabilities/external_issue_links.rb
  • spec/factories/vulnerabilities/feedback.rb
  • spec/factories/vulnerabilities/finding_identifiers.rb
  • spec/factories/vulnerabilities/finding_links.rb
  • spec/factories/vulnerabilities/finding_pipelines.rb
  • spec/factories/vulnerabilities/findings.rb
  • spec/factories/vulnerabilities/historical_statistics.rb
  • spec/factories/vulnerabilities/identifiers.rb
  • spec/factories/vulnerabilities/issue_links.rb
  • spec/factories/vulnerabilities/scanners.rb
  • spec/factories/vulnerabilities/statistics.rb
  • spec/finders/security/pipeline_vulnerabilities_finder_spec.rb
  • spec/lib/gitlab/ci/reports/security/aggregated_report_spec.rb
  • spec/lib/gitlab/ci/reports/security/vulnerability_reports_comparer_spec.rb
  • spec/models/vulnerabilities/export_spec.rb
  • spec/models/vulnerabilities/external_issue_link_spec.rb
  • spec/models/vulnerabilities/feedback_spec.rb
  • spec/models/vulnerabilities/finding_identifier_spec.rb
  • spec/models/vulnerabilities/finding_link_spec.rb
  • spec/models/vulnerabilities/finding_pipeline_spec.rb
  • spec/models/vulnerabilities/finding_remediation_spec.rb
  • spec/models/vulnerabilities/finding_spec.rb
  • spec/models/vulnerabilities/historical_statistic_spec.rb
  • spec/models/vulnerabilities/identifier_spec.rb
  • spec/models/vulnerabilities/issue_link_spec.rb
  • spec/models/vulnerabilities/projects_grade_spec.rb
  • spec/models/vulnerabilities/remediation_spec.rb
  • spec/models/vulnerabilities/scanner_spec.rb
  • spec/models/vulnerabilities/stat_diff_spec.rb
  • spec/models/vulnerabilities/statistic_spec.rb
  • spec/policies/vulnerabilities/export_policy_spec.rb
  • spec/policies/vulnerabilities/feedback_policy_spec.rb
  • spec/policies/vulnerabilities/issue_link_policy_spec.rb
  • spec/policies/vulnerabilities/scanner_policy_spec.rb
  • spec/serializers/vulnerabilities/feedback_entity_spec.rb
  • spec/serializers/vulnerabilities/finding_entity_spec.rb
  • spec/serializers/vulnerabilities/finding_reports_comparer_entity_spec.rb
  • spec/serializers/vulnerabilities/finding_serializer_spec.rb
  • spec/serializers/vulnerabilities/identifier_entity_spec.rb
  • spec/serializers/vulnerabilities/request_entity_spec.rb
  • spec/serializers/vulnerabilities/response_entity_spec.rb
  • spec/serializers/vulnerabilities/scanner_entity_spec.rb
  • spec/services/ci/compare_security_reports_service_spec.rb
Additional files to change/move based on Mehmet's suggestion

These files will also need to be changed/moved.

  • ee/app/services/security/merge_reports_service.rb
  • ee/lib/gitlab/ci/parsers/security/common.rb
  • ee/spec/lib/gitlab/ci/parsers/security/common_spec.rb
  • ee/spec/services/security/merge_reports_service_spec.rb

Difficulties

  • It may not be possible to break down into separate chunks
  • Scope of the changes may not be fully known
  • File locations/names, etc. will change over time, potentially making some of the info in this Issue outdated

Storing reports high level flow (default branch only)

  1. CI job scans project, generates, and uploads gl-sast-report.json job artifact
  2. When pipeline finishes, a background job is started. This worker merges all of the pipeline's jobs security reports together to deduplicate findings.
graph LR
A[CI SAST job runs scan] -->|Upload gl-sast-report.json| B(Deserialize to POROs)
B --> C{Security::StoreReportService saves to DB}
C -->D[INSERT INTO vulnerability_occurrences]
C -->E[INSERT INTO vulnerabilities]
C -->F[INSERT INTO vulnerability_scanners]
C -->G[INSERT INTO vulnerability_finding_pipelines]
C -->H[INSERT INTO vulnerability_identifiers]
C -->I[INSERT INTO ...]

callstack

notes

Displaying MR reports

graph LR
A[MergeRequestController#sast_reports fetches added/fixed findings]
A --> C{Security::CompareSecurityReportsService}
C -->D[Security::PipelineVulnerabilitiesFinder deserializes JSON to POROs]
C -->E[VulnerabilityReportsComparer diffs source and target reports]

callstack

  • Projects::MergeRequestsController#sast_reports # Controller endpoint
    • MergeRequests # ActiveRecord model
      • Ci::CompareSecurityReportsService # Returns hash of "added" and "fixed" findings to diff in the frontend
        • Security::PipelineVulnerabilitiesFinder # Finder service for normalizing JSON into POROs
        • Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer # Performant comparison between reports

Migration path

Phase 1 (starts June 7th)

  1. Move POROs into CE (ee/lib/gitlab/ci/**)
    1. Everything under ee/lib/gitlab/ci/reports/security to lib/gitlab/ci/reports/security (except non SAST locations)
    2. Gitlab::Ci::Parsers::Security::Common
    3. Gitlab::Ci::Parsers::Security::Sast and Gitlab::Ci::Parsers reference
  2. Move Security::MergeReportsService to the app/services/security

Phase 2 (starts July 12th)

  1. Move the Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer to the CE directory level

Phase 3 (starts August 2nd)

  1. Move the Ci::CompareSecurityReportsService to the app/services/ci/ directory
  2. (AS NEEDED) Move the bulk of these classes into CE with some EE-only features remaining in EE.
    1. Vulnerabilities::FindingDiffSerializer && FindingEntity with associate models
    2. Security::PipelineVulnerabilitiesFinder
  • Vulnerability should remain under EE
  • Finding should remain under CE
Edited by Lucas Charles