Skip to content

Add filtering by state to PipelineSecurityReportFindingsResolver

What does this MR do?

This MR introduces a new argument for the GraphQL resolver PipelineSecurityReportFindingsResolver.

Related to #331724 (closed).

Domain information

  • Security Finding: These are the vulnerability findings populated by the security analyzers and they are stored in JSON files attached to Ci::JobArtifacts.

  • Vulnerability: These are the vulnerabilities already existing in the default branch of the project and stored in the database.

  • Filtering by the state of a finding:

    • State of a security finding: Since these records are coming from JSON files, we determine their states on-the-fly by checking if they already have an associated vulnerability records. To do so, this MR introduces an extra SQL query to eager load the related vulnerability records from the database.
    • Filtering: After calculating the state of each finding, we iterate through all the records to filter them in-memory.

    For better understanding, the pseudo-code to calculate the state of a finding and then filtering based on that would be;

      # calculate the state of finding objects
      findings.each do |finding|
        if finding.vulnerability
          finding.state = finding.vulnerability.state
        else
          finding.state = 'detected'
        end
      end
    
      # filter by state
      findings.select { |finding| finding.state == SELECTED_STATE }

Database review

There is just one database-related change in this MR which preloads the vulnerability reords from the database for the findings by using the ActiveRecord::QueryMethods#includes(!64762 (diffs)).

An example of that query is;

SELECT
    "vulnerabilities".*
FROM
    "vulnerabilities"
WHERE
    "vulnerabilities"."id" IN (?)

And the execution plan for such query;

Index Scan using vulnerabilities_pkey on public.vulnerabilities  (cost=0.43..30.21 rows=10 width=294) (actual time=0.058..0.092 rows=10 loops=1)
   Index Cond: (vulnerabilities.id = ANY ('{1,2,3,4,5,6,7,8,9,10}'::bigint[]))
   Buffers: shared hit=44
   I/O Timings: read=0.000 write=0.000

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Mehmet Emin INAC

Merge request reports

Loading